Bug 224404

Summary: graphics/qgis: Update to 2.18.15
Product: Ports & Packages Reporter: Rainer Hurling <rhurlin>
Component: Individual Port(s)Assignee: Guido Falsi <madpilot>
Status: Closed FIXED    
Severity: Affects Only Me CC: madpilot, rhurlin
Priority: --- Flags: rhurlin: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch to update graphics/qgis from 2.18.14 to 2.18.15
rhurlin: maintainer-approval+
patch to update graphics/qgis to 2.18.15 with correct removal of /tmp/srs.db rhurlin: maintainer-approval+

Description Rainer Hurling freebsd_committer freebsd_triage 2017-12-17 16:51:59 UTC
Created attachment 188915 [details]
patch to update graphics/qgis from 2.18.14 to 2.18.15

Minor update from QGIS 2.18.14 to 2.18.15.

Changes of the port:
- Update to 2.18.15
- Update pkg-plist
- Before deleting /tmp/srs.db, 
  save its content into ${DATADIR}/resources

The patch was tested on Poudriere (10.3i/a, 11.1i/a and, HEADi/a) and 
portlint -AC seems happy.
Comment 1 Guido Falsi freebsd_committer freebsd_triage 2017-12-28 18:57:00 UTC
I'm testing the update.

Poudriere tells me you are missing USES+=pgsql when I compile with all options turned on. Most probably some option causes this, I'm investigating which. I suspect it's "POSTGIS".

Apart from that I have a doubt about the /tmp/srs.db file.

I see in the log the upstream installation already puts such a file in share/qgis/resources/srs.db, but you overwrite it with the one from /tmp. Are you sure the two are not the same?

Since this is a complicated port I'll be performing more tests.
Comment 2 Guido Falsi freebsd_committer freebsd_triage 2017-12-29 15:16:14 UTC
(In reply to Guido Falsi from comment #1)

> Apart from that I have a doubt about the /tmp/srs.db file.
> 
> I see in the log the upstream installation already puts such a file in
> share/qgis/resources/srs.db, but you overwrite it with the one from /tmp.
> Are you sure the two are not the same?

Regarding this file, I also discovered that when compiling the port with all options disabled, there is no /tmp/srs.db file to be copied, causing the port to fail to install.

I'm unable to guess which option triggers it's creation though.
Comment 3 Rainer Hurling freebsd_committer freebsd_triage 2017-12-30 07:27:56 UTC
(In reply to Guido Falsi from comment #2)

> Poudriere tells me you are missing USES+=pgsql when I compile with all
> options turned on. Most probably some option causes this, I'm
> investigating which. I suspect it's "POSTGIS".

Yes, Poudriere complains about missing USES+=pgsql, but it installs PostgreSQL because of the PostGIS dependency. I took this route, because there was a long discussion about one year ago on the mailing list, about how to avoid some unneeded and unwanted dependencies. I don't remember the exact discussion any more, but leaving out USES+=pgsql solves the problem, as several users reported back.

> Apart from that I have a doubt about the /tmp/srs.db file.
> 
> I see in the log the upstream installation already puts such a file in
> share/qgis/resources/srs.db, but you overwrite it with the one from /tmp.
> Are you sure the two are not the same?

No, they two files are not the same. The one from /tmp is newer and a bit bigger. I suspect, it is the one patched by QGIS, to correct some bugs of the original version.

> Regarding this file, I also discovered that when compiling the port with
> all options disabled, there is no /tmp/srs.db file to be copied, causing
> the port to fail to install.

Oops, this should not happen, my fault. I think, I must put it in the context of an option.
Comment 4 Guido Falsi freebsd_committer freebsd_triage 2018-01-18 22:29:20 UTC
Hi,

Have you had time to update the patch to solve the problem?
Comment 5 Rainer Hurling freebsd_committer freebsd_triage 2018-01-19 14:47:15 UTC
Created attachment 189906 [details]
patch to update graphics/qgis to 2.18.15 with correct removal of /tmp/srs.db

Hi Guido,

Sorry for the late response. I was very busy the last two weeks and finding out, which option produces the /tmp/srs.db file, was a bit time intensive.

It turns out, that /tmp/srs.db is only created by the test files (option TESTS enabled) and has nothing to do with the one in the resources directory. So in case of a test build, it should be removed afterwards. 

I filed a complete new patch. It should respect the test case correctly.

Thanks again for the hint.
Comment 6 commit-hook freebsd_committer freebsd_triage 2018-01-21 16:55:31 UTC
A commit references this bug:

Author: madpilot
Date: Sun Jan 21 16:55:24 UTC 2018
New revision: 459618
URL: https://svnweb.freebsd.org/changeset/ports/459618

Log:
  - Update qgis to 2.18.15
  - Fix TEST option

  PR:		224404
  Submitted by:	Rainer Hurling <rhurlin@gwdg.de>  (maintainer)

Changes:
  head/graphics/qgis/Makefile
  head/graphics/qgis/distinfo
  head/graphics/qgis/pkg-plist
Comment 7 Guido Falsi freebsd_committer freebsd_triage 2018-01-21 16:56:55 UTC
Committed, with minor changes.

I modified your patch too take advantage of option helper targets. This avoids having to split inclusion of bsd.port.mk.

I also added back the POSTGIS_USES=pgsql, it is reported as missing by stage-qa.

Thanks!
Comment 8 Rainer Hurling freebsd_committer freebsd_triage 2018-01-21 17:23:54 UTC
(In reply to Guido Falsi from comment #7)

Thanks, Guido, for the commit and the post-install-TESTS-on target. I wasn't aware of it. Very nice.

Even if Poudriere complains, I am not sure, if the reactivation of POSTGIS_USES=pgsql is really necessary. It builds without it and some users were happy about not having this line, because it brings in some extra dependencies they do not need and want. But anyway, we will see, if these users ask again for it ;)
Comment 9 Guido Falsi freebsd_committer freebsd_triage 2018-01-21 19:55:54 UTC
(In reply to Rainer Hurling from comment #8)
> (In reply to Guido Falsi from comment #7)
> 
> Thanks, Guido, for the commit and the post-install-TESTS-on target. I wasn't
> aware of it. Very nice.
> 
> Even if Poudriere complains, I am not sure, if the reactivation of
> POSTGIS_USES=pgsql is really necessary. It builds without it and some users
> were happy about not having this line, because it brings in some extra
> dependencies they do not need and want. But anyway, we will see, if these
> users ask again for it ;)

Uhm, this needs further analysis then.

Poudriere says this(before adding back the POSTGIS_USES=pgsql line):

Error: /usr/local/bin/qgis is linked to /usr/local/lib/libpq.so.5 from databases/postgresql95-client but it is not declared as a dependency
Warning: you need USES+=pgsql

it means the postgresql-client package has already been depended upon by some dependency and installed, and that the qgis binary is actually linking to it. So it IS required. We need to understand exactly which option is triggering this.

I thought that t was the POSTGIX option, but now I see the dependency on postgresql-client is actually brought in by the py-psycopg2 dependency in the PYTHON option.

So most probable the correct thing is to have a PYTHON_USES=pgsql. In such a case no dependency would be added.

Are you ok with such addition?

I'm going to do some testing around this idea to make sure that's the correct action.
Comment 10 Rainer Hurling freebsd_committer freebsd_triage 2018-01-21 20:15:53 UTC
> Poudriere says this(before adding back the POSTGIS_USES=pgsql line):
> 
> Error: /usr/local/bin/qgis is linked to /usr/local/lib/libpq.so.5
> from databases/postgresql95-client but it is not declared as a
> dependency
> Warning: you need USES+=pgsql
Yes, but in my Poudriere setting it was only handled as a warning and the build continues. Perhaps there is something wrong in my Poudriere config?!

> it means the postgresql-client package has already been depended
> upon by some dependency and installed, and that the qgis binary is
> actually linking to it. So it IS required. We need to understand
> exactly which option is triggering this.
> 
> I thought that t was the POSTGIX option, but now I see the
> dependency on postgresql-client is actually brought in by the
> py-psycopg2 dependency in the PYTHON option.
> 
> So most probable the correct thing is to have a PYTHON_USES=pgsql.
> In such a case no dependency would be added.
> 
> Are you ok with such addition?
It sounds reaasonable. On the other hand, using QGIS without Python does not make much sense. I am not aware of any user, who disabled the Python option. Without disabling it, the PostgreSQL dependency remains ...

> I'm going to do some testing around this idea to make sure
> that's the correct action.
Many thanks for your investigations. If no new problems will occur with PYTHON_USES=pgsql, feel free to put it in instead of POSTGIS_USES=pgsql :)
Comment 11 Guido Falsi freebsd_committer freebsd_triage 2018-01-21 20:40:40 UTC
(In reply to Rainer Hurling from comment #10)
> > Poudriere says this(before adding back the POSTGIS_USES=pgsql line):
> > 
> > Error: /usr/local/bin/qgis is linked to /usr/local/lib/libpq.so.5
> > from databases/postgresql95-client but it is not declared as a
> > dependency
> > Warning: you need USES+=pgsql
> Yes, but in my Poudriere setting it was only handled as a warning and the
> build continues. Perhaps there is something wrong in my Poudriere config?!
> 

Same here, it is a warning, but it cannot be simply ignored. If a binary links with a library that library IS a dependency of that binary and should be registered in the port.

The only exception is the case of overlinking (build system of the port linking with libraries which are actually unused), which should anyway be fixed.

> > it means the postgresql-client package has already been depended
> > upon by some dependency and installed, and that the qgis binary is
> > actually linking to it. So it IS required. We need to understand
> > exactly which option is triggering this.
> > 
> > I thought that t was the POSTGIX option, but now I see the
> > dependency on postgresql-client is actually brought in by the
> > py-psycopg2 dependency in the PYTHON option.
> > 
> > So most probable the correct thing is to have a PYTHON_USES=pgsql.
> > In such a case no dependency would be added.
> > 
> > Are you ok with such addition?
> It sounds reaasonable. On the other hand, using QGIS without Python does not
> make much sense. I am not aware of any user, who disabled the Python option.
> Without disabling it, the PostgreSQL dependency remains ...
> 
> > I'm going to do some testing around this idea to make sure
> > that's the correct action.
> Many thanks for your investigations. If no new problems will occur with
> PYTHON_USES=pgsql, feel free to put it in instead of POSTGIS_USES=pgsql :)

After a first test build I'm not too sure of my analysis. I need further tests. I'll let you know.
Comment 12 Guido Falsi freebsd_committer freebsd_triage 2018-01-21 22:22:02 UTC
(In reply to Guido Falsi from comment #11)
> (In reply to Rainer Hurling from comment #10)
> > > Poudriere says this(before adding back the POSTGIS_USES=pgsql line):
> > > 
> > > Error: /usr/local/bin/qgis is linked to /usr/local/lib/libpq.so.5
> > > from databases/postgresql95-client but it is not declared as a
> > > dependency
> > > Warning: you need USES+=pgsql
> > Yes, but in my Poudriere setting it was only handled as a warning and the
> > build continues. Perhaps there is something wrong in my Poudriere config?!
> > 
> 
> Same here, it is a warning, but it cannot be simply ignored. If a binary
> links with a library that library IS a dependency of that binary and should
> be registered in the port.
> 
> The only exception is the case of overlinking (build system of the port
> linking with libraries which are actually unused), which should anyway be
> fixed.
> 
> > > it means the postgresql-client package has already been depended
> > > upon by some dependency and installed, and that the qgis binary is
> > > actually linking to it. So it IS required. We need to understand
> > > exactly which option is triggering this.
> > > 
> > > I thought that t was the POSTGIX option, but now I see the
> > > dependency on postgresql-client is actually brought in by the
> > > py-psycopg2 dependency in the PYTHON option.
> > > 
> > > So most probable the correct thing is to have a PYTHON_USES=pgsql.
> > > In such a case no dependency would be added.
> > > 
> > > Are you ok with such addition?
> > It sounds reaasonable. On the other hand, using QGIS without Python does not
> > make much sense. I am not aware of any user, who disabled the Python option.
> > Without disabling it, the PostgreSQL dependency remains ...
> > 
> > > I'm going to do some testing around this idea to make sure
> > > that's the correct action.
> > Many thanks for your investigations. If no new problems will occur with
> > PYTHON_USES=pgsql, feel free to put it in instead of POSTGIS_USES=pgsql :)
> 
> After a first test build I'm not too sure of my analysis. I need further
> tests. I'll let you know.

I am now sure it's the POSTGIS option causing qgis to link against postgresql-client. This happens without enabling the pgsql uses. I tested enabling only that option. While enabling only the PYTHON option does not trigger it.

If it is able to link to the library it means the dependency is anyway being pulled in. In fact it's called by the unconditional USE_QT4=sql-pgsql.

The USES pgsql adds a dependency on the same port added by sql-pgsql, and does not add any further dependencies. It is actually required in that case. Maybe the information about adding extra dependencies is old?
Comment 13 Rainer Hurling freebsd_committer freebsd_triage 2018-01-22 06:34:58 UTC
> I am now sure it's the POSTGIS option causing qgis to link
> against postgresql-client. This happens without enabling
> the pgsql uses. I tested enabling only that option. While
> enabling only the PYTHON option does not trigger it.
Ok, then we do not have to switch to PYTHON_USES=pgsql.

> If it is able to link to the library it means the
> dependency is anyway being pulled in. In fact it's called
> by the unconditional USE_QT4=sql-pgsql.
And this option is really needed.

> The USES pgsql adds a dependency on the same port added by
> sql-pgsql, and does not add any further dependencies. It
> is actually required in that case. Maybe the information
> about adding extra dependencies is old?
Yes, my correspondence with users about pgsql dependencies is older than a year.

As result of your analysis, I think we should keep the POSTGIS_USES=pgsql as it is now in the port. What do you think?
Comment 14 Guido Falsi freebsd_committer freebsd_triage 2018-01-22 07:44:45 UTC
(In reply to Rainer Hurling from comment #13)

> > The USES pgsql adds a dependency on the same port added by
> > sql-pgsql, and does not add any further dependencies. It
> > is actually required in that case. Maybe the information
> > about adding extra dependencies is old?
> Yes, my correspondence with users about pgsql dependencies is older than a
> year.
> 
> As result of your analysis, I think we should keep the POSTGIS_USES=pgsql as
> it is now in the port. What do you think?

Exactly my thought, so I'm closing this again. Thanks!