Re: Patch for pgAdmin4 package on Mac OS X

From: Sandeep Thakkar <sandeep(dot)thakkar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Hamid Quddus <hamid(dot)quddus(at)enterprisedb(dot)com>
Subject: Re: Patch for pgAdmin4 package on Mac OS X
Date: 2016-05-31 17:37:51
Message-ID: CANFyU95VN6meqnHAERq21V4QSuVsLEEvw_QDpDXBM6n9nvxxcA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

On Tue, May 31, 2016 at 9:29 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi,
>
> On the first run, I get:
>
> App completed: /Users/dpage/git/pgadmin4/pkg/mac/../../mac-build/pgAdmin
> 4.app
> ./pkg/mac/create-dmg.sh: line 6: cd: dist: No such file or directory
> Cleaning up
> Copying data into temporary directory
> cp: ./../mac-build/pgAdmin 4.app: No such file or directory
> create-dmg.sh failed
> make: *** [appbundle] Error 1
>
> If I run it again, I get:
>
> App completed: /Users/dpage/git/pgadmin4/pkg/mac/../../mac-build/pgAdmin
> 4.app
> ./pkg/mac/create-dmg.sh: line 6: cd: dist: No such file or directory
> Directory ./pgadmin4-1.0-dev.dmg.src already exists. Please delete it
> manually.
> create-dmg.sh failed
> make: *** [appbundle] Error 1
>
> If I manually create $SRC/dist, it's much happier.
>
> I have used variable for dist. It will create the directory if doesn't
exist. The clean-appbunde will also remove the .src directory in dist just
in case it is present.

Other issues:
>
> - Your changes to the runtime don't seem to help. I've been staring at
> the code for an hour or so now, and I can't see the issue though. We
> may need some fresh eyes.
>
> yeah, I also spent more time to see the code and test it. Infact, I could
hardly reproduce it 1 or 2 times.

> - pkg/mac/create-dmg.sh is mixing upper and lower case variable names
> and with/without _, e.g. $dmgname vs $DMG_NAME
>
> not sure, why I used lower case, I prefer upper case always. I have made
the changes now to use upper case for all variables.

> - Shoudn't DMG_NAME be initialised to `grep "^APP_NAME" web/config.py
> | cut -d"=" -f2 | sed "s/'//g"` ?
>
> yes, I have used the couple of more variables that gives proper
understanding.

Thanks! I have attached the updated patch. (runtime changes remains same).

>
> On Tue, May 31, 2016 at 1:19 PM, Sandeep Thakkar
> <sandeep(dot)thakkar(at)enterprisedb(dot)com> wrote:
> > Somehow the patch skipped the Makefile changes. Attached is the updated
> > patch.
> >
> > On Mon, May 30, 2016 at 6:00 PM, Sandeep Thakkar
> > <sandeep(dot)thakkar(at)enterprisedb(dot)com> wrote:
> >>
> >> Thanks. I have fixed all the issues.
> >>
> >> Regarding the app not running from within the DMG for the first time, I
> >> was unable to reproduce it on Zilan's machine which didn't have the
> >> development env. On Murali's machine, it was reproducible for 1 time
> after
> >> couple of attempts.
> >>
> >> So, I just added the sync statement after settings the pythonpath value
> in
> >> the settings. May be this will resolve the issue. Please confirm.
> >> settings.sync();
> >>
> >> Attached is the updated patch. Thanks.
> >>
> >> On Fri, May 27, 2016 at 9:28 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >>>
> >>> On Fri, May 27, 2016 at 4:48 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >>> >
> >>> >
> >>> > On Fri, May 27, 2016 at 1:11 PM, Sandeep Thakkar
> >>> > <sandeep(dot)thakkar(at)enterprisedb(dot)com> wrote:
> >>> >>
> >>> >> Sure. Thanks.
> >>> >>
> >>> >> There is a typo in pkg/mac/build.sh. i.e
> >>> >>
> >>> >> s/HTML_HELP/HELP_PATH/
> >>> >>
> >>> >>
> >>> >> On Fri, May 27, 2016 at 5:26 PM, Dave Page <dpage(at)pgadmin(dot)org>
> wrote:
> >>> >>>
> >>> >>> Not entirely - we definitely need to improve it. I'll review the
> code
> >>> >>> as
> >>> >>> it is now though.
> >>> >
> >>> >
> >>> > OK, review time :-)
> >>> >
> >>> > - The appbundle name should be created from APP_NAME.app in
> config.py,
> >>> > e.g.
> >>> > 'pgAdmin 4.app'
> >>> >
> >>> > - The DMG name should be created from
> >>> > to_lower(remove_spaces(APP_NAME-APP_VERSION)) in config.py, e.g.
> >>> > pgadmin4-1.0-dev.dmg
> >>> >
> >>> > - Use #ifdef Q_OS_MAC in the QT code for Mac-specific code. There's
> no
> >>> > need
> >>> > to define another macro.
> >>> >
> >>> > - Please add "MINIFY_HTML = False" to config_local.py (and have
> Paresh
> >>> > do
> >>> > the same on his packages). This works around a code issue with the
> docs
> >>> > that
> >>> > I'll log a bug for.
> >>> >
> >>> > - In testing, I found that running the app from within the DMG
> doesn't
> >>> > seem
> >>> > to work the first time - it prompts for the path, then exits. Once I
> >>> > save
> >>> > the path it offers, it's fine on subsequent runs.
> >>> >
> >>> > - Once copied to my laptop, I saw the same issue as above.
> >>> >
> >>> > Once these issues are resolved, I think we're good to commit.
> >>>
> >>> Oh, a couple more things:
> >>>
> >>> - There should not be a copy of the app bundle in dist/ following the
> >>> build. Only the dmg should be there.
> >>>
> >>> - I think mac-build/ should be removed following a successful build,
> >>> rather than waiting for make clean (please talk to Paresh - his code
> >>> should do the same).
> >>>
> >>> --
> >>> Dave Page
> >>> Blog: http://pgsnake.blogspot.com
> >>> Twitter: @pgsnake
> >>>
> >>> EnterpriseDB UK: http://www.enterprisedb.com
> >>> The Enterprise PostgreSQL Company
> >>
> >>
> >>
> >>
> >> --
> >> Sandeep Thakkar
> >>
> >
> >
> >
> > --
> > Sandeep Thakkar
> >
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
Sandeep Thakkar

Attachment Content-Type Size
pgadmin-mac-appbundle-jun01.patch application/octet-stream 21.4 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-31 17:57:27 pgAdmin 4 commit: Fixes #1278 - Load the foreign server node javascript
Previous Message Dave Page 2016-05-31 15:59:47 Re: Patch for pgAdmin4 package on Mac OS X