Skip to content

Remove pkgconf spkg #40204

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

Merged
merged 11 commits into from
Jun 25, 2025
Merged

Remove pkgconf spkg #40204

merged 11 commits into from
Jun 25, 2025

Conversation

dimpase
Copy link
Member

@dimpase dimpase commented Jun 3, 2025

pkgconf, a.k.a. pkg-config, is available on all systems we support - even on the "naked" (no homebrew/macports) macOS one can install a formally certified/notarised package https://github.com/donmccaughey/pkg-config_pkg - so there is no reason to keep it in the tree.

There are big advantages to have pkg-config available at configure time, as recognition of several crucial external spkgs, such as (open)blas, zlib, etc. hinges upon pkg-config.
With this PR in, we proceed to remove them.

Last but not the least, it simplifies the Makefile by getting rid of base target, which becomes empty

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@dimpase dimpase requested review from tobiasdiez and jhpalmieri June 3, 2025 23:48
Copy link

github-actions bot commented Jun 4, 2025

Documentation preview for this PR (built with commit f48be88; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@tobiasdiez
Copy link
Contributor

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is green, so this looks good to me.

It looks like there are some unrelated changes here in planarity, or are these coming from another dependency PR?

@dimpase
Copy link
Member Author

dimpase commented Jun 5, 2025

right, the planarity thing comes from the fact that I cannot build Sage without it - it's from a positively reviewed PR #40153 which should be in the coming beta. I forgot to list it in the deps, done now.

@dimpase dimpase requested a review from tobiasdiez June 5, 2025 15:36
dimpase added 7 commits June 8, 2025 13:28
long overdue - also, supports gcc-15 out of the box
removed compile checks, as they were testing for a very old
planarity version, and keeping it needs figuring out the header
version to use.
@dimpase dimpase force-pushed the remove_pkg_config branch from ae8e869 to fc407a4 Compare June 8, 2025 18:50
@dimpase
Copy link
Member Author

dimpase commented Jun 17, 2025

fixed a typo leading to docbuild - should be OK now (it builds locally)

@tobiasdiez
Copy link
Contributor

Small nitpick: the CI https://github.com/sagemath/sage/actions/runs/15716873483/job/44288781941?pr=40204#step:11:497 complains about

real_configure: WARNING: unrecognized options: --with-system-bzip2, --with-system-pkgconf

I think these flags come from

SYSTEM_CONFIGURE_ARGS+=" --with-system-${SPKG}=${WITH_SYSTEM_SPKG}"

If this is something that could be fixed easily, I would prefer if its done as part of this PR - if not, also no biggy.

@dimpase
Copy link
Member Author

dimpase commented Jun 18, 2025

@tobiasdiez are these not coming from a previous generation of a docker image, and would go away in the next beta?

I tried to see where ${SPKG}, in the line you point at, would come from, and just don't find pkgconf anywhere suspected.

@tobiasdiez
Copy link
Contributor

Sounds reasonable. It's sad that you cannot really trust the CI 🙄.

Thanks for checking though!

vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 21, 2025
sagemathgh-40204: Remove pkgconf spkg
    
`pkgconf`, a.k.a. `pkg-config`, is available on all systems we support -
even on the "naked" (no homebrew/macports) macOS one can install a
formally certified/notarised package
https://github.com/donmccaughey/pkg-config_pkg - so there is no reason
to keep it in the tree.

There are big advantages to have pkg-config available at configure time,
as recognition of several crucial external spkgs, such as (open)blas,
zlib, etc. hinges upon pkg-config.
With this PR in, we proceed to remove them.

Last but not the least, it simplifies the Makefile by getting rid of
`base` target, which becomes empty


## 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
- sagemath#40011 - remove bzip2 spkg.
- sagemath#40153 - planarity spkg fix (can't build otherwise)
    
URL: sagemath#40204
Reported by: Dima Pasechnik
Reviewer(s): Dima Pasechnik, Tobias Diez
@vbraun vbraun merged commit 0a45763 into sagemath:develop Jun 25, 2025
26 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants