Re: Patch for pgAdmin4 RPM package - Mailing list pgadmin-hackers
| From | Sandeep Thakkar |
|---|---|
| Subject | Re: Patch for pgAdmin4 RPM package |
| Date | |
| Msg-id | CANFyU96uUAGHvTLOj_XcK3hC-DqzrpxrWxvPFfFC6J6WjKH29A@mail.gmail.com Whole thread Raw |
| In response to | Re: Patch for pgAdmin4 RPM package (Dave Page <dpage@pgadmin.org>) |
| Responses |
Re: Patch for pgAdmin4 RPM package
(Sandeep Thakkar <sandeep.thakkar@enterprisedb.com>)
|
| List | pgadmin-hackers |
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@pgadmin.org> wrote:
[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
Sandeep Thakkar
Attachment
pgadmin-hackers by date: