Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#29003 closed task (fixed)

Install Sage-specific .pc files when running make, not configure

Reported by: embray Owned by:
Priority: major Milestone: sage-9.1
Component: build Keywords:
Cc: dimpase, mkoeppe Merged in:
Authors: Erik Bray Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: c7fbc65 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

As noted in this comment, if we generate any .pc files at configure time, rather than write them directly to $SAGE_LOCAL it would be preferable to write them somewhere in the sage source tree instead, then copy them into the relevant install target (i.e. $SAGE_LOCAL) only when running make.

One tricky aspect to this is at least some SPGKs require our generated .pc files to be installed in $SAGE_LOCAL to build properly. Currently, one way around that is to add $(PCFILES) to the package's dependencies. This is needed at the very least for numpy.

A different workaround might be to modify PKG_CONFIG_PATH to include .pc files in the source tree. I have not tried that yet.

Change History (13)

comment:1 Changed 3 years ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/build/configure/pcfiles
  • Commit set to d5c952012a003739833265351bc0d977c84f39bb

First attempt at this seems to work.

I chose src/lib/pkgconfig as the path for .pc files to install from the source tree, as it has a nice congruence with src/bin. But that was just a minor preference--this path could be anywhere in the source tree, or even outside it (e.g. for VPATH builds, should we ever get that working...)


New commits:

16a0a75Trac #29001: Add workaround for older version of pkg-config that do not include the PKG_CHECK_VAR macro
d5c9520Create generated .pc files in the source tree and install them later instead of creating them directly in SAGE_LOCAL

comment:2 Changed 3 years ago by git

  • Commit changed from d5c952012a003739833265351bc0d977c84f39bb to 4cc7f45d219c7b0c8927f63ddd35bdfdea4d6fca

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4cc7f45Create generated .pc files in the source tree and install them later instead of creating them directly in SAGE_LOCAL

comment:3 follow-up: Changed 3 years ago by git

  • Commit changed from 4cc7f45d219c7b0c8927f63ddd35bdfdea4d6fca to e59f9912f91f0033d0f4a73d165212b4a373311f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

854c899Trac #29003: Create generated .pc files in the source tree and install them later instead of creating them directly in SAGE_LOCAL
62ac3ccTrac #29003: Also output generated gsl.pc to SAGE_SRC
e59f991Trac #29003: Instead of making individual packages responsible for .pc files being installed, just include them as part of the standard 'toolchain'

comment:4 in reply to: ↑ 3 Changed 3 years ago by embray

  • Status changed from new to needs_review

Replying to git:

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

854c899Trac #29003: Create generated .pc files in the source tree and install them later instead of creating them directly in SAGE_LOCAL
62ac3ccTrac #29003: Also output generated gsl.pc to SAGE_SRC
e59f991Trac #29003: Instead of making individual packages responsible for .pc files being installed, just include them as part of the standard 'toolchain'

I think the approach in this version might be simplest, and is closer to the existing behavior. The major difference now is that actually copying .pc files into $SAGE_LOCAL is deferred until make is run, rather than writing to $SAGE_LOCAL directly when running configure.

comment:5 Changed 3 years ago by git

  • Commit changed from e59f9912f91f0033d0f4a73d165212b4a373311f to c7fbc6582240c5b960e56a7d2af0e2bfb8ad3c25

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

90e81adTrac #29003: Also output generated gsl.pc to SAGE_SRC
c7fbc65Trac #29003: Instead of making individual packages responsible for .pc files being installed, just include them as part of the standard 'toolchain'

comment:6 Changed 3 years ago by dimpase

  • Reviewers set to Dima Pasechnik

testing

comment:7 Changed 3 years ago by dimpase

  • Status changed from needs_review to positive_review

OK, good, thanks!

comment:8 Changed 3 years ago by vbraun

  • Branch changed from u/embray/build/configure/pcfiles to c7fbc6582240c5b960e56a7d2af0e2bfb8ad3c25
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 3 years ago by dimpase

  • Commit c7fbc6582240c5b960e56a7d2af0e2bfb8ad3c25 deleted

there is sometimes a permission problem, shouldn't that cp -P used to install these things be replaced by something more robust.

see e.g. https://trac.sagemath.org/ticket/29071#comment:8

comment:10 Changed 3 years ago by dimpase

  • Cc mkoeppe added

by right, these *.pc files are package data, so they ought to be treated as such, no?

comment:11 Changed 3 years ago by dimpase

$(PCFILES) must be cleanable, by make clean I guess. Pleadse confirm. It caused a lot of pain on #29071

comment:12 Changed 3 years ago by dimpase

the followup (cleaning of *.pc files) is on #29082

comment:13 Changed 3 years ago by mkoeppe

... hotfix in #29121

Note: See TracTickets for help on using tickets.