Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
embray opened this issue Jan 13, 2020 · 21 comments
Closed

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

embray opened this issue Jan 13, 2020 · 21 comments

Comments

@embray
Copy link
Contributor

embray commented Jan 13, 2020

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.

CC: @dimpase @mkoeppe

Component: build

Author: Erik Bray

Branch: c7fbc65

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29003

@embray embray added this to the sage-9.1 milestone Jan 13, 2020
@embray
Copy link
Contributor Author

embray commented Jan 13, 2020

Branch: u/embray/build/configure/pcfiles

@embray
Copy link
Contributor Author

embray commented Jan 13, 2020

Author: Erik Bray

@embray
Copy link
Contributor Author

embray commented Jan 13, 2020

Commit: d5c9520

@embray
Copy link
Contributor Author

embray commented Jan 13, 2020

comment:1

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 13, 2020

Changed commit from d5c9520 to 4cc7f45

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 13, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 13, 2020

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'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 13, 2020

Changed commit from 4cc7f45 to e59f991

@embray
Copy link
Contributor Author

embray commented Jan 13, 2020

comment:4

Replying to @sagetrac-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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 14, 2020

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'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 14, 2020

Changed commit from e59f991 to c7fbc65

@dimpase
Copy link
Member

dimpase commented Jan 14, 2020

comment:6

testing

@dimpase
Copy link
Member

dimpase commented Jan 14, 2020

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Jan 14, 2020

comment:7

OK, good, thanks!

@vbraun
Copy link
Member

vbraun commented Jan 20, 2020

Changed branch from u/embray/build/configure/pcfiles to c7fbc65

@dimpase
Copy link
Member

dimpase commented Jan 24, 2020

comment:9

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. #29071 comment:8

@dimpase
Copy link
Member

dimpase commented Jan 24, 2020

Changed commit from c7fbc65 to none

@dimpase
Copy link
Member

dimpase commented Jan 24, 2020

comment:10

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

@dimpase
Copy link
Member

dimpase commented Jan 26, 2020

comment:11

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

@dimpase
Copy link
Member

dimpase commented Jan 26, 2020

comment:12

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 31, 2020

comment:13

... hotfix in #29121

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants