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

Do not stage .pc files in src/lib/, clean old generated *.pc files at 'make distclean', fix 'permission denied' errors #29082

Closed
dimpase opened this issue Jan 26, 2020 · 79 comments

Comments

@dimpase
Copy link
Member

dimpase commented Jan 26, 2020

#29003 introduced 2-stage installation of generated *.pc files,
via src/lib/pkgconfig- but did not provide a way to clean them up at all. And this is a problem (cf. e.g. #29071).

Also, in some installations, repeated installations this code leads to 'permission denied' errors when trying to overwrite read-only files:

  [openblas-0.3.6.p0]   Wrote /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/var/tmp/sage/build/openblas-0.3.6.p0/inst/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/lib/pkgconfig/blas.pc
  [openblas-0.3.6.p0]   Wrote /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/var/tmp/sage/build/openblas-0.3.6.p0/inst/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/lib/pkgconfig/cblas.pc
  [openblas-0.3.6.p0]   Wrote /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/var/tmp/sage/build/openblas-0.3.6.p0/inst/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/lib/pkgconfig/lapack.pc
  [openblas-0.3.6.p0]   
  [openblas-0.3.6.p0]   real	8m40.259s
  [openblas-0.3.6.p0]   user	45m2.739s
  [openblas-0.3.6.p0]   sys	5m48.754s
  [openblas-0.3.6.p0]   Copying package files from temporary location /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/var/tmp/sage/build/openblas-0.3.6.p0/inst to /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local
  [openblas-0.3.6.p0]   cp: /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/./lib/pkgconfig/cblas.pc: Permission denied
  [openblas-0.3.6.p0]   cp: /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/./lib/pkgconfig/blas.pc: Permission denied
  [openblas-0.3.6.p0]   cp: /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/.tox/local-homebrew-minimal/local/./lib/pkgconfig/lapack.pc: Permission denied

This ticket revises the installation as follows:

  • configure no longer creates .pc files in the build tree
  • Instead it creates rules in make/Makefile that create the .pc files in SAGE_LOCAL.
  • Before pc files are installed into SAGE_LOCAL, the targets are removed, to avoid permission problems

This ticket does not solve everything. There may still be problems when switching from a system library to an spkg-provided library. We will use the follow-up ticket #29387 (Complete solution for installing the generated *.pc files) to address this problem.

CC: @embray @mkoeppe @jhpalmieri @vbraun @kiwifb

Component: build: configure

Author: Matthias Koeppe

Branch/Commit: 329843b

Reviewer: Dima Pasechnik

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

@dimpase dimpase added this to the sage-9.1 milestone Jan 26, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:2

My suggestion would be to rewrite this as a script package instead of putting things into src/. This stuff really does not belong into src/.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:3

Added #29071 as a dependency as it touches the same files.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

Dependencies: #29071

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title clean generated *.pc files Move .pc file from src/ to build/, clean generated *.pc files at 'make distclean' Jan 28, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:5

(deleted a comment that was intended for #29071.)

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:6

(deleted a comment that was intended for #29071.)

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

comment:7

Replying to @mkoeppe:

My suggestion would be to rewrite this as a script package instead of putting things into src/. This stuff really does not belong into src/.

Perhaps we should just create var/ next to src/ and put these things there?

@dimpase

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:8

No, I don't think so. These are configure-generated files of sage-the-distribution - like build/make/Makefile. There's no need for a new top level.

Let me take care of this ticket after #29071 is done.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 29, 2020

Changed dependencies from #29071 to #29051, #29071, #29084

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 29, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 29, 2020

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 29, 2020

Last 10 new commits:

683a926make installing *.pc files conditional
a0acbcbcorrect quoting of m4 index variable
5ee6844ensure gfortran is available
56541aewrap AC_FC_FUNC so that it does not throw an error without Fortran
9d1770arevert #29025, remove useless [ALL] section
dba7aefrevert from "libraries=" back to section-specific "BLAH_libs="
3a4524eMerge remote-tracking branch 'trac/public/packages/numpy/no_DEFAULT_and_ALL_numpy_site_cfg' into openblaspcfix
06f46ebAdd blas to fflas_ffpack linked libraries so that openblas is picked up on Arch
5f16988Merge branch 'u/arojas/make_fflas_ffpack_detect_and_use_system_openblas_on_arch' of git://trac.sagemath.org/sage into t/29082/move__pc_file_from_src__to_build___clean_generated___pc_files_at__make_distclean_
2a01db1Move all BLAS PC file installation logic to new script package sage_sage_system_blas_facade

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 29, 2020

Commit: 2a01db1

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 29, 2020

comment:13

Branch is on top of #29051, #29071, #29084.
This is a first version, haven't tested much yet.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 29, 2020

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 30, 2020

Changed dependencies from #29051, #29071, #29084 to #29051, #29071, #29084, #26130, #29056

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 31, 2020

comment:16

I will redo this on top of the hot fix - #29121.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

comment:56

Error with gsl.pc for local-homebrew-macos-standard - https://github.com/mkoeppe/sage/runs/524361197

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

comment:57

also ubuntu-bionic-standard (sagelib and cvxopt)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

2a38a16build/pkgs/gsl/spkg-configure.m4: Quoting fix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2020

Changed commit from f5b4f62 to 2a38a16

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member Author

dimpase commented Mar 22, 2020

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Mar 22, 2020

comment:62

OK, this does the needed job. Perhaps the make rules could be made less explicit, but OK.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 22, 2020

comment:63

Thanks!

@mkoeppe mkoeppe changed the title Move .pc file from src/ to build/, clean generated *.pc files at 'make distclean', fix 'permission denied' errors Do not stage .pc files in src/lib/, clean old generated *.pc files at 'make distclean', fix 'permission denied' errors Mar 22, 2020
@vbraun
Copy link
Member

vbraun commented Mar 23, 2020

comment:65

Make distclean deletes src/lib/pkgconfig/.gitignore

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 23, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

329843bIgnore /src/lib/pkgconfig using top-level .gitignore

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 23, 2020

Changed commit from 2a38a16 to 329843b

@dimpase
Copy link
Member Author

dimpase commented Mar 24, 2020

comment:68

ok. sorry for overlooking this.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 24, 2020

comment:69

Thanks

@vbraun
Copy link
Member

vbraun commented Mar 29, 2020

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