Re: Patch for pgAdmin4 RPM package

Hi

Few changes in the updated patch:
- added the missing modules in the specfile. The unavailable modules are
still commented.
- added changelog in specfile
- added dependency of pgadmin4-doc on pgadmin4-web

On Wed, Jun 1, 2016 at 2:57 PM, Sandeep Thakkar <
sandeep(dot)thakkar(at)enterprisedb(dot)com> wrote:

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

--
Sandeep Thakkar