MIME-Version: 1.0
In-Reply-To: 
 <CANFyU97Ofo-fTF5HmNqrfX=i6a=6eJayksNq=W2Ay2yhT4ig5g@mail.gmail.com>
References: 
 <CANFyU96tOSdX00+ZjUZ++wF9qzLt9UE_YdKjDWVzHGik+5kB4w@mail.gmail.com>
	<CA+OCxoxBDt82P75Q4Cy4fBkznkPBW3YJwPz+wFARbBBSoi3fgg@mail.gmail.com>
	<CANFyU97Ofo-fTF5HmNqrfX=i6a=6eJayksNq=W2Ay2yhT4ig5g@mail.gmail.com>
Date: Fri, 27 May 2016 14:05:52 +0100
Message-ID: 
 <CA+OCxoxgLB-mE=-_qHsQbBbziJ2D+h9qVRx9Am7hgrgpc52gkw@mail.gmail.com>
Subject: Re: Patch for pgAdmin4 RPM package
From: Dave Page <dpage@pgadmin.org>
To: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>,
	Hamid Quddus <hamid.quddus@enterprisedb.com>,
 =?UTF-8?B?RGV2cmltIEfDnE5Ew5xa?= <devrim@gunduz.org>
Content-Type: text/plain; charset=UTF-8
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

[Adding Devrim]

On Fri, May 27, 2016 at 1:55 PM, Sandeep Thakkar
<sandeep.thakkar@enterprisedb.com> wrote:
>
>
> On Mon, May 9, 2016 at 6:35 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> Initial eyeball review comments below...
>>
>> On Fri, Apr 22, 2016 at 11:57 AM, Sandeep Thakkar
>> <sandeep.thakkar@enterprisedb.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


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers