Re: Patch for pgAdmin4 RPM package

Hi Devrim,

I have attached the patch for review. Please note that right now I have
commented the python dependencies (in Requires) which you are building.
Please review and let me know if specfile or anything else needs some
changes. Once the rpms are built, please let me know how to install them so
that I will enable those dependencies and do the testing.

Hi Dave,

The rpm will be built in $SRC/rpm-build. Inside this, we have the
directories for sources (where tarball will be downloaded - for testing, I
have mentioned the path of Bugatti :) ), build, etc.

The html docs was not building and I had to make changes in docs/conf.py
and install sphinx_rtd_theme. I have added this dependency and the Sphinx
in the specfile. May be should add it in the requirements also? I tested
this change on OS X and make docs is building fine.

Since web package is installed in the default python site-packages as
pgadmin4-web-v1 (for release "1"), with the help of Neel, I could made
changes in Server.cpp to find that location. But, couldn't understand how
to get the app release info, hence right now hard-coded the string as
'pgadmin4-web-v1".

Note: In the patch, the Makefile and .gitignore also contains the mac
related changes. This is just to see how they will look finally after mac
and rpm changes are done. I will remove them from the rpm patch once the
mac appbundle patch is committed.

Questions:
1. Should we add 'docs' dependency target for 'rpm' like we did for
appbundle?
2. Should web rpm require doc rpm? I guess so, otherwise online help won't
work. Right?

On Fri, May 27, 2016 at 6:35 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> [Adding Devrim]
>
> On Fri, May 27, 2016 at 1:55 PM, Sandeep Thakkar
> <sandeep(dot)thakkar(at)enterprisedb(dot)com> wrote:
> >
> >
> > On Mon, May 9, 2016 at 6:35 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >>
> >> Hi
> >>
> >> Initial eyeball review comments below...
> >>
> >> On Fri, Apr 22, 2016 at 11:57 AM, Sandeep Thakkar
> >> <sandeep(dot)thakkar(at)enterprisedb(dot)com> wrote:
> >>>
> >>> Hi Team, Dave,
> >>>
> >>> Attached herewith are two patches.
> >>>
> >>> pgadmin4-rpm.patch - This is the main patch that includes scripts,
> >>> makefiles and spec to create RPMs for RHEL6/RHEL7/F-22/F-23/F-24.
> >>
> >>
> >> Can we keep the directory names in lower case?
> >>
> >
> > Sure. Will do that.
> >>>
> >>> It will create two RPMs i.e pgadmin4 and pgadmin4-web. The pgadmin4
> tpm
> >>> depends on web and the web rpm depends on the python packages. I have
> >>> commented the list of packages which are not available on some systems
> so
> >>> that Devrim can build them.
> >>>
> >>> The installation path for pgadmin4 is "/usr/pgadmin4-<major>.<minor>"
> and
> >>> pgadmin4-web is the site-packages/pgadmin4-web
> >>
> >> Shouldn't the -web package also have the major.minor version number in
> the
> >> path, to allow side-by-side installation?
> >
> > Right. Now that we don't have major/minor, so, will it be
> /usr/pgadmin4-v1
> > and pgadmin4-web-v1 ? Or?
>
> I think that's fine.
>
> >>
> >>
> >>>
> >>> pgadmin4-server-ini.patch - This is the patch for runtime/Server.cpp.
> As
> >>> said pgadmin4-web and runtime installation directories are different
> and
> >>> that means web does not exists in parallel to runtime like in sources.
> >>>
> >>> I observed that the location of application settings was not defined in
> >>> Server.cpp. As per QSettings doc, the default location on Unix is the
> >>> $HOME/.config/<companyname>/<appname>.conf. Here, $HOME depends on the
> user
> >>> that runs the application. So, I thought why not to define the
> application
> >>> settings in application directory itself. RPM then knows where to
> define the
> >>> ApplicationPath. I tested it and it worked fine with me. I haven't
> done this
> >>> change for platform dependent.
> >>
> >> Doesn't that prevent non-root users from changing the settings? Or (if
> you
> >> widen the permissions on the ini file), allow one user to mis-configure
> the
> >> app for others? I think what is needed here is a search path change,
> much
> >> like you added for the Mac app bundle.
> >>
> > Right. Will use python command to find the site-packages path and then
> > concatenate pgadmin4-web directory name.
>
> OK.
>
> >> Other thoughts:
> >>
> >> - Please rename the README to README.txt
> >>
> >> - The code to build the RPMs should be entirely confined to pkg/rpm. A
> >> Makefile target should be added to /Makefile to build/clean the targets
> >> (this mistake was made with the Mac package too, but was one of the
> original
> >> requirements).
> >>
> >> Please resolve these issues and I'll take another look.
> >>
> > Sure. Will share it with you soon.
>
> -> Devrim please :-)
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
Sandeep Thakkar