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

From: Victoria Henry <vhenry(at)pivotal(dot)io>
To: Dave Page <dpage(at)pgadmin(dot)org>
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-08 14:49:20
Message-ID: CANxYE3Km4jNv50Q7RrAtx9ig8fDug=G1-jp7BS-XXuwJ5aCZBQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.

>
>
- 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'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.

>
> - 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.

> - 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/pwf0k2d509s2xx6vh0h633vm0000gn/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/pwf0k2d509s2xx6vh0h633vm0000gn/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.

Thanks,

Victoria & Joao

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Victoria Henry 2018-06-08 18:43:37 [pgAdmin][patch] RM3409 - Retrieving SQL from table throws exception
Previous Message Dave Page 2018-06-08 09:08:42 Re: [pgadmin4][Patch]: Test cases for the backup module