Re: Patch for pgAdmin4 RPM package

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?

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

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

> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
Sandeep Thakkar