Opened 23 months ago

Last modified 21 months ago

#29082 closed defect

Move .pc file from src/ to build/, clean generated *.pc files at 'make distclean' — at Version 26

Reported by: dimpase Owned by:
Priority: blocker Milestone: sage-9.1
Component: build: configure Keywords:
Cc: embray, mkoeppe, jhpalmieri, vbraun, fbissey Merged in:
Authors: Matthias Koeppe Reviewers:
Report Upstream: N/A Work issues:
Branch: u/mkoeppe/move__pc_file_from_src__to_build___clean_generated___pc_files_at__make_distclean_ (Commits, GitHub, GitLab) Commit: d6b5677f0a51dd790377482f4608fd041fb0fbbe
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

#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). This ticket revises the 2-stage installation as follows:

  • Because the generated files are for sage-the-distribution, not sagelib, they belong into build, not src.
  • Because configure (config.status) creates them, make distclean cleans them.
  • Code in build/make/Makefile.in is removed in favor of creating a new type=script package sage_system_blas_facade, whose spkg-install does the installation to SAGE_LOCAL.
  • openblas/spkg-configure.m4 sets SAGE_BLAS=sage_system_blas_facade if PC files are generated.

Change History (26)

comment:1 Changed 23 months ago by dimpase

  • Cc mkoeppe added
  • Status changed from new to needs_info

comment:2 follow-up: Changed 23 months ago by 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/.

comment:3 Changed 23 months ago by mkoeppe

  • Dependencies set to #29071
  • Description modified (diff)
  • Summary changed from clean generated *.pc files to Move .pc file from src/ to build/, clean generated *.pc files at 'make distclean'

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

comment:4 Changed 23 months ago by mkoeppe

  • Status changed from needs_info to needs_work

comment:5 follow-up: Changed 23 months ago by mkoeppe

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

Last edited 23 months ago by mkoeppe (previous) (diff)

comment:6 in reply to: ↑ 5 Changed 23 months ago by mkoeppe

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

Last edited 23 months ago by mkoeppe (previous) (diff)

comment:7 in reply to: ↑ 2 Changed 23 months ago by dimpase

  • Description modified (diff)

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?

comment:8 Changed 23 months ago by mkoeppe

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.

comment:9 Changed 23 months ago by mkoeppe

  • Dependencies changed from #29071 to #29051, #29071, #29084

comment:10 Changed 23 months ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:11 Changed 23 months ago by mkoeppe

  • Branch set to u/mkoeppe/move__pc_file_from_src__to_build___clean_generated___pc_files_at__make_distclean_

comment:12 Changed 23 months ago by mkoeppe

  • Commit set to 2a01db11ef106d00217d3176a794850524467bcd
  • Description modified (diff)

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

comment:13 Changed 23 months ago by mkoeppe

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

comment:15 Changed 23 months ago by mkoeppe

  • Dependencies changed from #29051, #29071, #29084 to #29051, #29071, #29084, #26130, #29056

comment:16 Changed 23 months ago by mkoeppe

  • Priority changed from blocker to major

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

comment:17 Changed 22 months ago by mkoeppe

  • Description modified (diff)

comment:18 Changed 22 months ago by git

  • Commit changed from 2a01db11ef106d00217d3176a794850524467bcd to 2535c186176468a691627bdc0cff84d8387782a8

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

2535c18Move all BLAS PC file installation logic to new script package sage_sage_system_blas_facade

comment:19 Changed 22 months ago by mkoeppe

  • Dependencies #29051, #29071, #29084, #26130, #29056 deleted
  • Status changed from needs_work to needs_review

Rebased on 9.1.beta4

comment:20 Changed 22 months ago by mkoeppe

  • Cc jhpalmieri added

comment:21 Changed 22 months ago by git

  • Commit changed from 2535c186176468a691627bdc0cff84d8387782a8 to 3f2060ebf71d3cd593efe44a751b1a8017af969b

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

3f2060eMerge tag '9.1.beta5' into t/29082/move__pc_file_from_src__to_build___clean_generated___pc_files_at__make_distclean_

comment:22 Changed 22 months ago by mkoeppe

  • Cc vbraun fbissey added

Needs review.

comment:23 Changed 22 months ago by git

  • Commit changed from 3f2060ebf71d3cd593efe44a751b1a8017af969b to d6b5677f0a51dd790377482f4608fd041fb0fbbe

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

d6b5677Merge tag '9.1.beta6' into t/29082/move__pc_file_from_src__to_build___clean_generated___pc_files_at__make_distclean_

comment:24 Changed 21 months ago by embray

Agreed on the first two points in the description.

Don't agree on sage_system_blas_facade and the changes to Makefile.in. I don't see how it's "special-purpose code". It's a quite generic solution that you seem to be replacing with "special-purpose code" that's very specific to openblas. This ticket was needed for more than just blas.

comment:25 Changed 21 months ago by embray

  • Status changed from needs_review to needs_info

comment:26 Changed 21 months ago by mkoeppe

  • Description modified (diff)
Note: See TracTickets for help on using tickets.