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

Adapt to pkgconf 1.9.4 #17

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Adapt to pkgconf 1.9.4 #17

wants to merge 10 commits into from

Conversation

ppisar
Copy link
Contributor

@ppisar ppisar commented Mar 3, 2023

This patchset adapts to changes in pkgconf 1.9.

Upstream stopped adding trailing spaces to cflags/libs strings. This patch puts them back as that would break a use case when one concatenates various flags for a command line.

Most complex part is dealing with changed pkgconf dependency solver. pkgconf now returns only direct cflags/libs. One needs to use a pkgconf solver which populates an in-client cache and stores intermediate results into a dedicated PKGCONF_PKG_PROPF_VIRTUAL package.

Because the solver ignores packages loaded from a file, this patch also temporarily injects these packages into a cache to retrieve flags including dependencies.

All tests pass with pkgconf 1.9.4 and 1.8.0.

#15

Fixes error handled prototype.
Adapts to passing flags to file_open.
Adapts tests to no traling spaces (incompatible change).
Works around a possible pkgconf regression with trailing null bytes.

TODO: t/package.t fails on missing shared libs from dependencies.
TODO: Restore compatibility with older pkgconfs.
Some t/package.t are fixed now.
But packages loaded with package_from_file() fails on the solver. See
t/package.t.
Solver works on cached packages. Packages are cached either manually,
or by exploring all installation paths. To obtain cflags/libs with the
solver we need to cache the packages loaded from files.

Drawbacks:

(1) pkgconf tool flushed the cache before retrieving nonstatic libs
("reset solver when solving for library groups"). I don't understand
why. I was unable to observe any effect. Because we stash the package
into a cache, we cannot drop the cache when retrieving the libs. Thus
this patch removes flushing the cache added in port to 1.9.4.

(2) The from-file-loaded package poisons the cache. When querying
other packages, the from-file-loaded package could be included in the
results. I'm not sure whether the original isolation was good or bad.
I can imagine someone wants to override a system-provided pkgconfig
file and thus loads the override from a file.

Alternatively, we could cache the package loaded from a file
temporarily just around using the solver. That would narrow the time
window when the package is in the cache.
This resolves all the remaining tests.
pkgconf-1.9 fixed cflags and libs values by removing the trailing
spaces. However, this consitutes a change in behaviour (people got
used to concatenate flags) and thus this patch returns the trailing
spaces.
This implements an alternative approach outlined in
d3efe46 commit.

This prevents from having packages loaded from files in a client-wide
cache all the time. The cache is used for resolving cflags/libs.

A downside is that the code is more complex.
@plicease
Copy link
Member

plicease commented Mar 3, 2023

Can you apply this patch:

diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml
index c6ec3e8..f6cfb22 100644
--- a/.github/workflows/linux.yml
+++ b/.github/workflows/linux.yml
@@ -17,11 +17,12 @@ jobs:
       fail-fast: false
       matrix:
         cip:
-          - tag: "5.35"
-          - tag: "5.34"
-          - tag: "5.34"
+          - tag: "5.37"
+          - tag: "5.36"
+          - tag: "5.36"
             install_from_git: 1
-          - tag: "5.34-fedora34"
+          - tag: "5.36-fedora34"
+          - tag: "5.34"
           - tag: "5.32"
           - tag: "5.30"
           - tag: "5.28"

that should fix the CI failure.

@ppisar
Copy link
Contributor Author

ppisar commented Mar 6, 2023

I pushed the CI patch. 5.36,1 fails on a wrong autoconf version and 5.36-fedora34 fails on a missing docker image.

Fedora 34 become unsupported a year ago. Now the oldest supported Fedora is 36. I recommend you instead of hard-coding a Fedora version, use fedora:latest for the latest stable Fedora, or fedora:rawhide for the latest developmental Fedora https://hub.docker.com/_/fedora/tags.

…ifier

Alien-Build-2.77 tests revealed a bug in constructing a query for
pkgconf-1.9 solver: If a package file had file name different from
a Name value inside the file, the package was able to be found, but
the flags solver searched for the Name and found nothing.

Studying pre-pkgconf documentation shows that Name value is only
a human-oriented display name and a base of the package file name
should be used instead as a package identifier. This base name is
stored into an id field of the package structure of pkgconf.

This patch fixes it by using the id field instead. It also adds a test
to prevent from future regressions.
@ppisar
Copy link
Contributor Author

ppisar commented Mar 7, 2023

Alien-Build tests found a mistake when retrieving flags for a package file whose Name field differs from its file name. At pushed a fix to the pull request.

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this pull request Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants