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

enable pc #1522

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

enable pc #1522

wants to merge 2 commits into from

Conversation

xnox
Copy link
Contributor

@xnox xnox commented Sep 27, 2024

  • generateRuntimePkgConfigDeps: only do so for public .pc, not vendored
    The existing neom test case of vendored .pc files verifies this, and
    fails, when this feature is turned on.

    The diff is ugly, but it moves and indents the if generateRuntimePkgConfigDeps{} to be nested in the if isInDir(){}
    first condition for public files.

  • Enable pc file dependencies
    Downloaded and extracted 1,030 .pc files from wolfi (base on 24th
    September file listing of all packages from @dustin).

    Executing pkgconf --exist on them revieled that 57 of them declare
    dependencies that do not, in fact exist.

    It is safe to turn pc: depends, as for vast majority of packages they
    will work correctly.

    For the 57 broken ones, there is now test/pkgconf pipeline, and one
    genuinely has to fix those. By either packaging libraries that are
    declared as required, ensuring those dependant libraries ship
    pkgconfig file, or patch/remove said broken pkgconfig file.

    Out of the sample of 1,030 .pc files, the 788 of them do have
    requirements declared. Thus they will need rebuilds to gain the
    correct dependencies. These however are in just 469 binary packages,
    some of which come from the same origin. Thus roughly at most 400 or
    so origins to rebuild.

    Furthermore wolfi pre-submit, install apk check will catch the broken
    -dev packages that cannot be installed.

    With this feature enabled, build log has:

    2024/09/27 23:44:24 INFO scanning for pkg-config data...
    2024/09/27 23:44:24 INFO   found pkg-config libarchive for usr/lib/pkgconfig/libarchive.pc
    ...
    2024/09/27 23:44:24 INFO   found pkg-config dependency (requires private) libcrypto for usr/lib/pkgconfig/libarchive.pc
    ...
    2024/09/27 23:44:24 INFO   runtime:
    2024/09/27 23:44:24 INFO     bzip2-dev
    2024/09/27 23:44:24 INFO     lz4-dev
    ...
    2024/09/27 23:44:24 INFO     pc:libcrypto
    ...
    2024/09/27 23:44:24 INFO     so:libarchive.so.13
    2024/09/27 23:44:24 INFO     xz-dev
    2024/09/27 23:44:24 INFO     zlib-dev
    2024/09/27 23:44:24 INFO     zstd-dev
    2024/09/27 23:44:24 INFO   provides:
    2024/09/27 23:44:24 INFO     pc:libarchive=3.7.6-r0
    

    These extra new dependency generated pc:libcrypto which upon
    installing libarchive-dev allows one to use it straight away for both
    dynamic and static linking.

    Note all of our public pc files already generated provides:

    $ apk info --provides openssl-dev
    openssl-dev-3.3.2-r0 provides:
    pc:libcrypto=3.3.2
    pc:libssl=3.3.2
    pc:openssl=3.3.2
    

The existing neom test case of vendored .pc files verifies this, and
fails, when this feature is turned on.

The diff is ugly, but it moves and indents the `if
generateRuntimePkgConfigDeps{}` to be nested in the `if isInDir(){}`
first condition for public files.
Downloaded and extracted 1,030 .pc files from wolfi (base on 24th
September file listing of all packages from @dustin).

Executing `pkgconf --exist` on them revieled that 57 of them declare
dependencies that do not, in fact exist.

It is safe to turn pc: depends, as for vast majority of packages they
will work correctly.

For the 57 broken ones, there is now test/pkgconf pipeline, and one
genuinely has to fix those. By either packaging libraries that are
declared as required, ensuring those dependant libraries ship
pkgconfig file, or patch/remove said broken pkgconfig file.

Out of the sample of 1,030 .pc files, the 788 of them do have
requirements declared. Thus they will need rebuilds to gain the
correct dependencies. These however are in just 469 binary packages,
some of which come from the same origin. Thus roughly at most 400 or
so origins to rebuild.

Furthermore wolfi pre-submit, install apk check will catch the broken
-dev packages that cannot be installed.

With this feature enabled, build log has:

```
2024/09/27 23:44:24 INFO scanning for pkg-config data...
2024/09/27 23:44:24 INFO   found pkg-config libarchive for usr/lib/pkgconfig/libarchive.pc
...
2024/09/27 23:44:24 INFO   found pkg-config dependency (requires private) libcrypto for usr/lib/pkgconfig/libarchive.pc
...
2024/09/27 23:44:24 INFO   runtime:
2024/09/27 23:44:24 INFO     bzip2-dev
2024/09/27 23:44:24 INFO     lz4-dev
...
2024/09/27 23:44:24 INFO     pc:libcrypto
...
2024/09/27 23:44:24 INFO     so:libarchive.so.13
2024/09/27 23:44:24 INFO     xz-dev
2024/09/27 23:44:24 INFO     zlib-dev
2024/09/27 23:44:24 INFO     zstd-dev
2024/09/27 23:44:24 INFO   provides:
2024/09/27 23:44:24 INFO     pc:libarchive=3.7.6-r0
```

These extra new dependency generated `pc:libcrypto` which upon
installing libarchive-dev allows one to use it straight away for both
dynamic and static linking.

Note all of our public pc files already generated provides:

```
$ apk info --provides openssl-dev
openssl-dev-3.3.2-r0 provides:
pc:libcrypto=3.3.2
pc:libssl=3.3.2
pc:openssl=3.3.2
```
@mattmoor
Copy link
Member

mattmoor commented Oct 1, 2024

Very cool.

A drive-by comment: historically every time we touch SCA we've broken.... something 😅 Now that we have world builds, were you able to use that to help/do any of the validation here?

I definitely appreciate the thoroughness outlined in your PR description, so thank you for that! 🙏

@xnox
Copy link
Contributor Author

xnox commented Oct 1, 2024

Very cool.

A drive-by comment: historically every time we touch SCA we've broken.... something 😅 Now that we have world builds, were you able to use that to help/do any of the validation here?

I definitely appreciate the thoroughness outlined in your PR description, so thank you for that! 🙏

for that I need to substitute a different sdk image / wolfictl /melage used during the build. So far I have used world rebuilds with custom packages / custom extra package repositories. But not with customized wolfictl / melange.

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.

2 participants