Re: [pgadmin][patch] Electron version 4.X

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Victoria Henry <vhenry(at)pivotal(dot)io>
Cc: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgadmin][patch] Electron version 4.X
Date: 2018-06-11 15:35:36
Message-ID: CA+OCxowDvRSEpEcV75-ZAsOgzXcJi+SM3jGac=ao16-RqQ4uuQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

On Fri, Jun 8, 2018 at 3:49 PM, Victoria Henry <vhenry(at)pivotal(dot)io> wrote:

>
>
> On Tue, Jun 5, 2018 at 12:28 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Mon, Jun 4, 2018 at 10:27 PM, Joao De Almeida Pereira <
>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached you can find the patch that introduces electron to our code
>>> base.
>>>
>> Cool. FYI, I'd like to get this into rough shape and then push it to a
>> dev branch for fine-tuning. I think it'll be easier to work that way.
>>
> Sure, you can create a dev branch and push this patch to it.
>
> Great. So here's my initial feedback:
>>
>> - The Mac build you provided doesn't work for me. It hangs on the loading
>> screen.
>>
> Could we get some more information about the machine? Using the python
> from the venv directory, are you able to run the app directly?
>
> cd /Applications/pgAdmin.app/Contents/Resources/app/
> venv/bin/python web/pgAdmin4.py
>
> ​
> Do you get and error?
> We are setting PGADMIN_PORT, PGADMIN_KEY and SERVER_MODE environment
> variables prior to starting it.
>

Yes, I get:

piranha:app dpage$ venv/bin/python web/pgAdmin4.py

Could not find platform independent libraries <prefix>

Could not find platform dependent libraries <exec_prefix>

Consider setting $PYTHONHOME to <prefix>[:<exec_prefix>]

Fatal Python error: Py_Initialize: unable to load the file system codec

ModuleNotFoundError: No module named 'encodings'

Current thread 0x00007fffe86993c0 (most recent call first):

Abort trap: 6

>
>
>>
>>
> - A number of the changes are related to the naming of requirejs. I'd be
>> inclined to pull that out into a separate patch and get it committed to
>> master ASAP.
>>
> This change only makes sense in the Electron context. Neverthless fell
> free to add it to master if you think it is relevant.
>
>
>> - I think the build instructions need to be more generic (particularly on
>> macOS). For example, I do not use HomeBrew (largely due to some nasty
>> security issues they had in the past). I was able to mostly port the
>> instructions and build script over to work using MacPorts (without PyEnv)
>> which actually turned out to be somewhat more simple than what's there now.
>>
> Since we don't use MacPorts, we cannot provide installation instructions.
>

I wasn't asking you to provide MacPort instructions; I can do that. I was
saying that the basic outline of the instructions should be agnostic to the
package tools the user might choose; e.g.

- Install python
- Create a virtual environment using the --copies option called venv
- Move the virtual environment to ...

We can then include examples of how to do that with brew, macports etc.

>
>
>> - I'm not sure what this is intended to do: "git checkout electron".
>> Clearly that isn't correct.
>>
> That was the name of our development branch. It can be removed.
>

OK.

>
>
>>
>> - All new builds should be using Python 3.6. We need to deprecate 2.7 as
>> there are some Unicode related issues that cannot be fixed in it.
>>
> For Windows, we are using 2.7 because of external library compilation
> issues. Let us know if you are able to get around this or how to make
> this work.
>

There are a couple of tricks if you're using Python 3.6. First, to compile
Pycrypto you need to do something like:

- set VCINSTALLDIR=C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC
- set CL=-FI"%VCINSTALLDIR%\INCLUDE\stdint.h"
- (https://stackoverflow.com/questions/41843266/
microsoft-windows-python-3-6-pycrypto-installation-error
<https://stackoverflow.com/questions/41843266/microsoft-windows-python-3-6-pycrypto-installation-error>
)

Beforehand. Then, edit Lib\site-packages\Crypto\Random\OSRNG\nt.py" and
modify " *import winrandom*" to "*from . import winrandom*".

As a sidenote, I use the VC++ 2015 Build Tools compiler from Microsoft,
without installing any SDKs or Visual Studio.

>
>
>> - I would like to see the new build code adapted to follow the existing
>> conventions as much as reasonable, to avoid having to change build systems
>> or other processes/procedures that folks use. For example, build scripts
>> should be under pkg/, completed packages left in dist/, build staging done
>> in xxx-build directories rather than elsewhere.
>>
> That sounds reasonable.
>
>
>> - It may be a result of my use of MacPorts, but I'm getting the following
>> failure building:
>>
>
>> yarn run v1.3.2
>> $ electron-forge make --platfrom=darwin --arch=x64 --targets=dmg
>> ✔ Checking your system
>> ✔ Resolving Forge Config
>> We need to package your application before we can make it
>> ✔ Preparing to Package Application for arch: x64
>> ⠼ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/venv/lib/python3.6/site-packages/setuptools/command/launcher
>> manifest.xml
>> Compiling /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/venv/lib/python3.6/site-packages/setuptools/command/launcher
>> manifest.xml resulted in a MIME type of application/xml, which we don't
>> know how to handle
>> ⠋ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/browser/server_groups/
>> servers/templates/servers/supported_servers.js
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/browser/server_groups/
>> servers/templates/servers/supported_servers.js: Unexpected token (6:7)
>> ⠦ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/browser/templates/
>> browser/js/endpoints.js
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/browser/templates/browser/js/endpoints.js:
>> Unexpected token (5:6)
>> Failed to compile file: /var/folders/c6/pwf0k2d509s2xx
>> 6vh0h633vm0000gn/T/electron-packager/darwin-x64/pgAdmin-
>> darwin-x64/Electron.app/Contents/Resources/app/web/
>> pgadmin/browser/templates/browser/js/messages.js
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/browser/templates/browser/js/messages.js:
>> Unexpected token (37:1)
>> Failed to compile file: /var/folders/c6/pwf0k2d509s2xx
>> 6vh0h633vm0000gn/T/electron-packager/darwin-x64/pgAdmin-
>> darwin-x64/Electron.app/Contents/Resources/app/web/
>> pgadmin/browser/templates/browser/js/utils.js
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/browser/templates/browser/js/utils.js:
>> Unexpected token (28:23)
>> ⠋ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/misc/file_manager/
>> templates/file_manager/js/languages/en.js
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/misc/file_manager/
>> templates/file_manager/js/languages/en.js: Unexpected token, expected ;
>> (2:29)
>> ⠋ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/static/scss/_alert.scss
>> Error: Undefined variable: "$color-green-3".
>> on line 29 of /stdin
>> >> background: $color-green-3;
>>
>> --------------^
>>
>> ⠙ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/static/scss/_alertify.overrides.scss
>> Error: ".ajs-message.ajs-error.ajs-visible" failed to @extend
>> ".bg-red-1".
>> The selector ".bg-red-1" was not found.
>> Use "@extend .bg-red-1 !optional" if the extend should be able to
>> fail.
>> on line 164 of /stdin
>> >> @extend .bg-red-1;
>>
>> ----------^
>>
>> ⠴ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/static/scss/sqleditor/_history.scss
>> Error: Undefined variable: "$color-gray-3".
>> on line 4 of /stdin
>> >> border-bottom: 1px solid $color-gray-3;
>>
>> -----------------------------^
>>
> We were also seeing these errors but they don't have any impact on the
> compilation and should be addressed in the future (not in this patch)
>
>
>>
>> ⠇ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/templates/base.html
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/templates/base.html:inline_1.js:
>> Unexpected token (2:13)
>> ⠴ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/tools/templates/js/translations.js
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/tools/templates/js/translations.js:
>> Unexpected token (2:10)
>> ⠦ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/tools/user_management/
>> templates/user_management/js/current_user.js
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vm0000gn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/web/pgadmin/tools/user_management/
>> templates/user_management/js/current_user.js: Unexpected token (3:15)
>> ✔ Compiling Application
>> ✔ Preparing native dependencies
>> ✔ Packaging Application
>> Making for the following targets:
>> ⠋ Making for target: dmg - On platform: darwin - For arch: x64
>> An unhandled exception has occurred inside Forge:
>> Cannot find module 'macos-alias'
>> Error: Cannot find module 'macos-alias'
>>
> This may be related to using MacPorts or the way you installed the
> dependencies. There was a bug with Electron and Node 10 so we had to use
> to Node 8 with Yarn.
>

I'm using Node 8 as well. Let's figure out your build first and ensure we
can get that working everywhere, then I'll try to figure out how to
reproduce it.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2018-06-11 15:56:44 pgAdmin 4 commit: Avoid an exception on GreenPlum when retrieving RE-SQ
Previous Message Dave Page 2018-06-11 15:01:26 Re: [pgadmin4][patch] Use pytest test runner for unit tests