Opened 8 years ago

Closed 8 years ago

#11765 closed defect (fixed)

Sage's 'pkg-config' directory should always be added to PKG_CONFIG_PATH

Reported by: leif Owned by: leif
Priority: blocker Milestone: sage-4.7.2
Component: scripts Keywords: pkgconfig .pc sage-env
Cc: fbissey, jhpalmieri Merged in: sage-4.7.2.rc1
Authors: Leif Leonhardy Reviewers: Steven Trogdon
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by leif)

Currently, PKG_CONFIG_PATH is only set to Sage's pkg-config directory ($SAGE_ROOT/local/lib/pkgconfig/) if it is empty (or not set at all).

The patch now also prepends Sage's directory otherwise, such that Sage's .pc files will be picked up first.

The previous behaviour could obviously break the build if PKG_CONFIG_PATH was already set in the "global" environment, i.e. outside the Sage environment.


Apply trac_11765-always_add_sages_pc_dir_in_sage-env.rebased.scripts.patch to the scripts repo.

Attachments (2)

trac_11765-always_add_sages_pc_dir_in_sage-env.scripts.patch (1.1 KB) - added by leif 8 years ago.
SCRIPTS repo. Based on Sage 4.7.2.alpha2.
trac_11765-always_add_sages_pc_dir_in_sage-env.rebased.scripts.patch (1.1 KB) - added by leif 8 years ago.
SCRIPTS repo. Same patch rebased on Sage 4.7.2.rc0.

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by leif

SCRIPTS repo. Based on Sage 4.7.2.alpha2.

comment:1 Changed 8 years ago by leif

  • Cc fbissey jhpalmieri added
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 8 years ago by fbissey

I looked at the patch and the reasoning is sound. I cannot test the patch properly right now but it looks correct to me.

comment:3 Changed 8 years ago by strogdon

  • Reviewers set to Steven Trogdon
  • Status changed from needs_review to positive_review

I encountered this pkg-config problem while building vanilla sage-4.7.2.rc0 in a gentoo prefix on two different debian machines. On a x86 machine the build broke at the R spkg and on an amd64 machine at the matplotlib spkg. In both situations the failure was due to an incompatibility resulting from trying to include headers for libpng15, my system libpng headers, instead of headers for the sage-installed libpng12. Applying this patch allowed the correct headers to be located and sage to be built on both machines. I give this a positive review. The patch may need to be rebased.

patching file sage-env
Hunk #2 succeeded at 261 (offset 7 lines).

comment:4 Changed 8 years ago by strogdon

Perhaps an explanation, which was pointed out to me by François, is in order as to why the patch is needed here. I had previously built sage-4.7.2.rc0 on a gentoo system without a problem. However, the build process broke when done from within a gentoo prefix. On gentoo PKG_CONFIG_PATH is not defined, by default, and thus there was no problem. But from within gentoo prefix PKG_CONFIG_PATH is defined. Thus the patch is needed there to prepend to PKG_CONFIG_PATH the sage paths so the correct .pc files can be found.

comment:5 follow-up: Changed 8 years ago by fbissey

  • Priority changed from critical to blocker

PKG_CONFIG_PATH is not defined on Gentoo full stop. Prefix or regular. So this may break sage in subtle ways. R-2.10.1 as shipped in sage may break in the build stage because it is not compatible with png-1.5.

I will elevate this to blocker and rebase it ASAP. Jeroen can decide whether it should be shipped in 4.7.2.

comment:6 in reply to: ↑ 5 Changed 8 years ago by leif

Replying to fbissey:

I will elevate this to blocker and rebase it ASAP.

Well, there's no fuzz, just a few lines offset, so you don't have to rebase the patch.

Changed 8 years ago by leif

SCRIPTS repo. Same patch rebased on Sage 4.7.2.rc0.

comment:7 follow-up: Changed 8 years ago by fbissey

Well thanks that saves me the work :)

comment:8 in reply to: ↑ 7 Changed 8 years ago by leif

  • Description modified (diff)

Replying to fbissey:

Well thanks that saves me the work :)

If it makes you happy... ;-)

comment:9 Changed 8 years ago by jdemeyer

  • Merged in set to sage-4.7.2.rc1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.