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

Remove php provides with a version, replace with providerPriority #34680

Closed
wants to merge 2 commits into from

Conversation

smoser
Copy link
Member

@smoser smoser commented Nov 20, 2024

In all the php-8.-.yaml files:

  1. replace all of the 'phpMM' definitions with simple var rather than transform based name.

  2. replace any virtual provides with a version with an empty version and set providerPriority to a value based on the PHP version (instead of package version).

    This takes care to handle PHP versions 8.10 by using major*100 + minor (803 for 8.3 , which would mean 810 for 8.10).

The first item isn't a huge win, but the second is needed if we're going to have virtual provides.

Fixes: #32874

In all the php-8.*-*.yaml files:
1. replace all of the 'phpMM' definitions with simple
   var rather than transform based name.
2. replace any virtual provides with a version with
   an empty version and set providerPriority to a
   value based on the PHP version (instead of package version).

   This takes care to handle PHP versions 8.10 by using
   major*100 + minor (803 for 8.3 , which would mean 810 for 8.10).

The first item isn't a huge win, but the second is needed
if we're going to have virtual provides.

Fixes: wolfi-dev#32874
@smoser
Copy link
Member Author

smoser commented Nov 20, 2024

We will need a withdrawal after these packages land of all the packages that had a provides.
This is the list of origin packages that had a phpPriority added:

php-8.1-amqp.yaml
php-8.1-igbinary.yaml
php-8.1-redis.yaml
php-8.2-amqp.yaml
php-8.3-amqp.yaml
php-8.3-apcu.yaml
php-8.3-excimer.yaml
php-8.3-grpc.yaml
php-8.3-igbinary.yaml
php-8.3-imagick.yaml
php-8.3-memcached.yaml
php-8.3-msgpack.yaml
php-8.3-opentelemetry.yaml
php-8.3-redis.yaml
php-8.3-swoole.yaml

@smoser
Copy link
Member Author

smoser commented Nov 20, 2024

This change was done like this:

php-cleanup:

#!/usr/bin/python3
import copy
import io
import os
import re
import sys
import subprocess
from ruamel.yaml import YAML

yaml2 = YAML()
mm = re.compile(r"php-(?P<maj>\d)[.](?P<min>\d)-.*[.]yaml")
changed = []
for fname in sys.argv[1:]:

    result = mm.search(fname)
    if not result:
        continue
    major = int(result.group("maj"))
    minor = int(result.group("min"))

    if not os.path.isfile(fname):
        debug(f"SKIP - {fname} not a file")
        continue

    with open(fname, "rb") as fp:
        content = fp.read()
    y = yaml2.load(content)
    if y is None:
        print(f"huh? {fname} y is None")
        raise RuntimeError(f"bad {fname}")

    pkg = y['package']['name']
    print("%s (%d.%d)" % (fname, major, minor))

    def fix_pkg_deps(pkg):
        found = False
        if 'dependencies' not in pkg:
            return False
        for n, v in enumerate(pkg['dependencies'].get('provides', [])):
            if '=${{package.full-version}}' not in v:
                continue
            found = True
            pkg['dependencies']['provides'][n] = v.split('=', 1)[0]

        if found:
            pkg['dependencies'].update(
                {"provider-priority": "${{vars.phpPriority}}"})

        return found

    hadprovides = fix_pkg_deps(y['package'])

    if 'subpackages' in y:
        for sp in y['subpackages']:
            hadprovides = any((hadprovides, fix_pkg_deps(sp)))

    varlines = [b'vars:', bytes(('  phpMM: "%s.%s"' % (major, minor)).encode())]

    if hadprovides:
        buf = io.BytesIO()
        yaml2.dump(y, buf)
        content = buf.getvalue()

        varlines.append(bytes(('  phpPriority: %d' % (major*100+minor)).encode()))

    start = content.find(b"\nvar-transforms:")
    end = content.find(b"\n\n", start)
    if start < 0 or end < 0:
        raise RuntimeError("no var-transofrm in %s" % fname)

    # jam the 'vars' in the same place 'var-transform' was.
    content = content[0:start+1] + b"\n".join(varlines) + content[end:]

    with open(fname, "wb") as fp:
        fp.write(content)

    changed.append(fname)

if len(changed):
    print('chagned hasa %d' % len(changed))
    subprocess.check_output(["yam"] + changed)
    subprocess.check_output(["wolfictl", "bump"] + changed)

then

$ php-cleanup php-8.**.yaml
$ sed -i -e 's,description: \([^"]*\)$,description: "\1",' -e 's,tag: v${{package.version}},tag: "v${{package.version}}",' $(git grep -l provider-priority php-8.*-*.yaml)

the sed was just to clean up some of the ruamel quotation changes.

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build log, there's no actual build failure - the build completed successfully. However, there is a warning during configure that might be worth addressing:

• Detected Warning: ./configure: line 6125: /usr/bin/file: not found

• Error Category: Configuration/Dependency

• Failure Point: Configure step, but non-fatal

• Root Cause Analysis: The file command is missing from the build environment, which is used during the configure process to detect binary formats and file types.

• Suggested Fix: Add file package to the build environment dependencies in the melange YAML:

environment:
  contents:
    packages:
      - autoconf
      - build-base
      - busybox
      - file           # Add this line
      - php-${{vars.phpMM}}
      - php-${{vars.phpMM}}-dev
      - rabbitmq-c-dev

• Explanation: While the build succeeds without file, having it available allows for proper binary format detection during configuration which can help prevent potential issues with shared library handling and binary compatibility.

• Additional Notes:

  • The overall build is successful and produces working packages
  • The warning doesn't impact functionality but addressing it follows best practices
  • All required dependencies for PHP AMQP extension compilation are present

• References:

This is a minor improvement that won't affect functionality but maintains cleaner build hygiene by ensuring all standard build tools are available.

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build log, I'll analyze the error and provide a solution:

• Detected Error: ./configure: line 6125: /usr/bin/file: not found

• Error Category: Dependency

• Failure Point: During the configure phase when checking for file command

• Root Cause Analysis: The build system is missing the file command which is required during the configure step to detect binary formats and dependencies. While the build completes successfully, this missing dependency should be addressed for proper system detection.

• Suggested Fix: Add file to the environment packages section in the melange YAML:

environment:
  contents:
    packages:
      - autoconf
      - build-base
      - busybox
      - file         # Add this line
      - php-${{vars.phpMM}}
      - php-${{vars.phpMM}}-dev
      - rabbitmq-c-dev

• Explanation: The file command is a standard Unix utility used during configure scripts to detect binary formats and library types. While the build completes without it in this case, including it will ensure proper system detection and prevent potential issues with binary compatibility checks.

• Additional Notes:

  • The package still builds successfully despite this warning
  • Adding file is considered best practice for configure scripts
  • This fix follows the principle of complete build environments
  • No other critical issues were found in the build log

• References:

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build output, I can provide an analysis and resolution:

• Detected Error: The build actually completed successfully - there are only compiler warnings, not errors:

/home/build/apc_cache.c: In function 'apc_cache_fetch':
/home/build/apc_cache.c:826:28: warning: variable 'entry' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]

• Error Category: Compilation Warnings (not errors)

• Failure Point: The build completed successfully, but with compiler warnings about potential variable clobbering

• Root Cause Analysis: These are standard compiler warnings about variables that could theoretically be clobbered during longjmp operations. These warnings don't affect functionality and are common in PHP extensions using Zend Engine.

• Suggested Fix: Add compiler flags to suppress these specific warnings:

environment:
  contents:
    packages:
      - autoconf
      - build-base
      - busybox
      - php-${{vars.phpMM}}
      - php-${{vars.phpMM}}-dev
  environment:
    CFLAGS: "-Wno-clobbered"

• Explanation:

  • The -Wno-clobbered flag will suppress these specific warnings
  • The warnings are not indicative of actual problems
  • This is a common pattern in PHP extensions due to how the Zend Engine handles exceptions
  • The build is actually successful and the package works correctly

• Additional Notes:

  • The package built and installed correctly
  • All tests passed successfully
  • The warnings are related to how GCC handles setjmp/longjmp which PHP uses internally
  • This is expected behavior for PHP extensions

• References:

The package is building successfully and working as intended - the warnings can be suppressed if desired but do not indicate any actual problems.

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build log, I'll analyze the error and provide a solution:

• Detected Error: ./configure: line 5959: /usr/bin/file: not found

• Error Category: Dependency

• Failure Point: Configure step during build process, specifically during library detection

• Root Cause Analysis: The 'file' command is missing from the build environment, which is needed during the configure process to identify binary file types and library characteristics

• Suggested Fix:
Add 'file' to the environment packages list in the melange YAML:

environment:
  contents:
    packages:
      - autoconf
      - build-base
      - busybox
      - file            # Add this line
      - mpdecimal-dev
      - php-${{vars.phpMM}}
      - php-${{vars.phpMM}}-dev

• Explanation:
The 'file' command is a critical utility used during the configure process to detect binary formats and library types. While the build completed despite this missing dependency, it's best practice to include it to ensure proper library detection and linking.

• Additional Notes:

  • Although the build completed successfully, missing the 'file' command could potentially cause issues with library detection in some cases
  • This is a common build dependency that should be included for proper configuration
  • The warning doesn't appear to have affected the final build output in this case, but fixing it is still recommended for reliability

• References:

The package still built successfully despite this warning, but adding the 'file' package will ensure more reliable builds and proper library detection during the configure phase.

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build output, I'll analyze the specific error and provide a fix:

• Detected Error: ./configure: line 5818: /usr/bin/file: not found

• Error Category: Dependency

• Failure Point: Configure script during build process

• Root Cause Analysis: The build system is missing the file command which is required by the configure script to detect file types and characteristics during the build process.

• Suggested Fix: Add file to the build environment dependencies in the melange YAML:

environment:
  contents:
    packages:
      - autoconf
      - build-base
      - busybox
      - file           # Add this line
      - php-${{vars.phpMM}}
      - php-${{vars.phpMM}}-dev

• Explanation: The file command is a standard Unix utility used by configure scripts to identify file types. While the build continues in this case, having the proper file type detection ensures more reliable builds and proper library/binary detection.

• Additional Notes:

  • Although the build completed successfully despite the missing file command, it's best practice to provide all required build utilities
  • The warning appears during the library detection phase of configure
  • Adding file ensures proper file type detection during configuration and build
  • This is a common requirement for autotools-based builds

• References:

The build was able to complete successfully in this case, but adding the file package will eliminate the warning and ensure proper file type detection during the build process.

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build log, while the package actually builds successfully, there appears to be an authentication error early in the process that should be addressed.

• Detected Error:

Error: rpc error: code = NotFound desc = federate identity: rpc error: code = NotFound desc = no identity found for (https://accounts.google.com, 109346087047205543085)
Error running `chainctl auth token`: exit status 1

• Error Category: Authentication/Configuration

• Failure Point: Early in the build process when attempting to authenticate

• Root Cause Analysis: The build is attempting to use Google authentication but cannot find valid credentials. However, this appears to be non-fatal as the build continues and completes successfully.

• Suggested Fix:
Add skip authentication configuration to the melange yaml:

environment:
  requirements:
    authentication: false
  contents:
    packages:
      # existing packages...

• Explanation:
The authentication error appears to be related to the CI system trying to authenticate with Google accounts, but this isn't actually required for building this package. Adding the authentication skip configuration will prevent this error while allowing the build to proceed normally as it already does.

• Additional Notes:

  • The actual package build succeeds completely
  • All compilation steps complete without errors
  • The final packages are generated successfully
  • This is more of a CI configuration issue than a package build issue
  • The authentication error can be safely ignored if you prefer, since it doesn't affect the build outcome

• References:

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

• Detected Error: ./configure: line 6172: /usr/bin/file: not found

• Error Category: Build Configuration

• Failure Point: During configure script execution

• Root Cause Analysis: The file command is missing from the build environment, which is needed during the configure phase to detect binary formats and library types.

• Suggested Fix: Add file to the build environment packages list:

environment:
  contents:
    packages:
      - autoconf
      - build-base
      - busybox
      - file           # Add this line
      - grpc-dev
      - php-${{vars.phpMM}}
      - php-${{vars.phpMM}}-dev

• Explanation: The file command is a standard Unix utility that determines file types. The configure script uses it to detect binary formats and library characteristics during the build configuration phase. While the build completed successfully despite this missing dependency, it's best practice to include it to ensure proper library detection and linking.

• Additional Notes:

  • While the build succeeded without file, including it follows best practices for configure scripts
  • The warning didn't cause build failure because the script had fallback detection methods
  • Including file may help with better binary format detection on different architectures

• References:

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build log, I don't actually see any errors - the build appears to have completed successfully. The build process:

  1. Successfully checked out the source code
  2. Ran phpize
  3. Configured the build
  4. Compiled the source files
  5. Installed the built files
  6. Generated the packages

The output shows three .apk packages were created:

  • php-8.1-igbinary-3.2.16-r3.apk
  • php-8.1-igbinary-config-3.2.16-r3.apk
  • php-8.1-igbinary-dev-3.2.16-r3.apk

And an index was generated successfully.

• Detected Error: None found - build completed successfully
• Error Category: N/A
• Failure Point: N/A
• Root Cause Analysis: N/A
• Suggested Fix: No fix needed as the build is working correctly
• Explanation: The build process completed all steps without errors and generated the expected package artifacts
• Additional Notes: If you're seeing failures in CI, they may be coming from a different part of the pipeline not shown in this log

If you're still experiencing issues, please provide the specific error message from the CI system as this build log shows a successful build.

Copy link
Contributor

octo-sts bot commented Nov 20, 2024

Gen AI suggestions to solve the build error:

Based on the build output, I don't actually see a build error - the package appears to have compiled and built successfully. The build completed with generating the APK packages and indexes without any errors.

Here's the analysis:

• Detected Error: None found - the build completed successfully
• Error Category: N/A
• Failure Point: N/A
• Root Cause Analysis: The build process completed all steps:

  • Source checkout
  • Configure
  • Make/compile
  • Install
  • Package generation
  • APK index creation

• Suggested Fix: No fix needed as the build succeeded

• Explanation: The build output shows:

  1. All compilation steps completed
  2. Shared objects were created
  3. Package dependencies were resolved
  4. APK packages were generated:
    • php-8.1-imagick-3.7.0-r4.apk
    • php-8.1-imagick-config-3.7.0-r4.apk

• Additional Notes:

  • There was one minor warning about "/usr/bin/file: not found" during configure but this did not impact the build
  • The build successfully linked against ImageMagick 7 libraries
  • OpenMP support was properly detected and enabled
  • All PHP extension requirements were met

• References:

  • Build log shows successful integration with ImageMagick 7.1.1-41
  • PHP 8.1.30 compatibility confirmed during configure

If you're seeing a CI failure, it may be in another part of the pipeline not shown in this build output. The package build itself appears successful.

@octo-sts octo-sts bot added the bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages. label Nov 20, 2024
php-8.1-igbinary.yaml Show resolved Hide resolved
@xnox
Copy link
Member

xnox commented Nov 20, 2024

Provides for humans work, if they are consistent:

$ ls php-8.[0-9]-*.yaml | wc -l
75

$ git grep -l provides php-8.[0-9]-*.yaml | wc -l
17

Out of 75 addons we have, only 17 have some provides. I wonder if we should drop provides all together; and make everyone be explicit as to what they want. Or add provides consistently everywhere else.

Removing provides is sort of a breaking change; thus likely if we choose to go that route, we should drop them with php8.4.

@smoser
Copy link
Member Author

smoser commented Nov 20, 2024

Out of 75 addons we have, only 17 have some provides. I wonder if we should drop provides all together; and make everyone be explicit as to what they want. Or add provides consistently everywhere else.

I had that thought too. my only reason for keeping them was that they'd be an indication of "the right way" to do it. (And if it wasn't actually the right way, then we'd fix it so it was).

@smoser smoser marked this pull request as draft November 20, 2024 14:29
@smoser
Copy link
Member Author

smoser commented Nov 20, 2024

I'm going to push another PR that drops the provides entirely.

smoser added a commit to smoser/wolfi-os that referenced this pull request Nov 20, 2024
Some of the php modules had virtual provides. The intent of such
a thing would be so that a process could 'apk add php-amqp' and
get a sane result.

The implementation had a problem where by doing so would choose
whichever php-8.X-amqp version had the newest package version.
That is almost certainly _not_ what the process wanted (one day to get
php-8.2-ampq and the next to get php-8.1-ampq).

wolfi-dev#32874 describes the problem
and a solution. wolfi-dev#34680 implemented
that solution.  In the PR we realized that the provides were so
inconsistent that it would be had to use the. So then, given the
option of making them consistent or dropping them, we chose to drop
them.
@smoser
Copy link
Member Author

smoser commented Nov 20, 2024

I'm going to close this PR, replacing it with #34720

@smoser smoser closed this Nov 20, 2024
smoser added a commit that referenced this pull request Nov 20, 2024
Some of the php modules had virtual provides. The intent of such a thing
would be so that a process could 'apk add php-amqp' and get a sane
result.

The implementation had a problem where by doing so would choose
whichever php-8.X-amqp version had the newest package version. That is
almost certainly _not_ what the process wanted (one day to get
php-8.2-ampq and the next to get php-8.1-ampq).

#32874 describes the problem and a
solution. #34680 implemented that
solution. In the PR we realized that the provides were so inconsistent
that it would be had to use the. So then, given the option of making
them consistent or dropping them, we chose to drop them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

php virtual provides should not include version
3 participants