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

Update melange convert python to have tests and produce subpackages for each python version. #1464

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

Conversation

justinvreeland
Copy link
Contributor

Melange Pull Request Template

Functional Changes

  • This change can build all of Wolfi without errors (describe results in notes)

Notes:

I have not tried to build all of Wolfi but this change shouldn't effect builds and the packages I have built with these changes build correctly. Including the results from the convert changes. (Though some python packages need some changes because they produce binaries or need C extensions built or something)

Additional Notes:

The pipeline that this convert script uses is in Wolfi's repo not this one. I'm not sure why that is but I'm happy to add it to this PR if that's desired.

This hardcodes python3.{10,11,12} for use in the generated yaml, it is not ideal but it would be nice if it was more configurable. Different melange users might want to specify a different python versions in particular I might want to have a file in Wolfi that specifies the supported python3 versions and reads that to generate the range if that file is found instead of using the the hardcoded set but my brief look at melange didn't find anything similar to model it off of.

@pnasrat
Copy link
Contributor

pnasrat commented Aug 30, 2024

It'd be helpful if you attached the output of a run of convert python with this in the PR for reference.

I'm also wondering if we should be doing this through a go template - see

var controlTemplate = `# Generated by melange
for an example to potentially support external templates. That might be a future change as I'm guessing the different versions of generate don't have a consistent struct that could be rendered.

@justinvreeland justinvreeland marked this pull request as draft August 30, 2024 20:25
@justinvreeland
Copy link
Contributor Author

It'd be helpful if you attached the output of a run of convert python with this in the PR for reference.

Will do.

I'm also wondering if we should be doing this through a go template - see

var controlTemplate = `# Generated by melange

for an example to potentially support external templates. That might be a future change as I'm guessing the different versions of generate don't have a consistent struct that could be rendered.

Certainly something to look into for future changes for sure. I hadn't noticed that when I was looking through things.

@justinvreeland justinvreeland force-pushed the jvreeland/update-melange-conversion-script branch from 382cc76 to 87605da Compare September 3, 2024 23:16
To do this add data and vars to use for substitution as well as a
subapckage and remove the unversioned python dep since it will be
generated correctly as part of the build.
@justinvreeland justinvreeland force-pushed the jvreeland/update-melange-conversion-script branch from 87605da to 444e7a0 Compare September 3, 2024 23:35
@justinvreeland
Copy link
Contributor Author

# Generated from https://pypi.org/project/packaging/
package:
  name: py3-packaging
  version: "24.1"
  epoch: 0
  description: Core utilities for Python packages
  copyright:
    - license: ""
  dependencies:
    provider-priority: "0"

environment:
  contents:
    packages:
      - build-base
      - busybox
      - ca-certificates-bundle
      - flit-core
      - py3-supported-pip
      - wolfi-base

pipeline:
  - uses: git-checkout
    with:
      expected-commit: 85442b8032cb7bae72866dfd7782234a98dd2fb7
      repository: https://github.com/pypa/packaging
      tag: v${{package.version}}

subpackages:
  - range: py-versions
    name: py${{range.key}}-${{vars.pypi-package}}
    pipeline:
      - name: Python Build
        uses: py/pip-build-install
        with:
          python: python${{range.key}}
    dependencies:
      provides:
        - py3-${{vars.pypi-package}}
      provider-priority: ${{range.value}}
    test:
      environment: {}
      pipeline:
        - name: Import Test
          uses: python/import
          with:
            import: ${{vars.module_name}}
            python: python${{range.key}}

data:
  - name: py-versions
    items:
      "3.10": "310"
      "3.11": "311"
      "3.12": "312"

update:
  enabled: true
  manual: false
  github:
    identifier: pypa/packaging

vars:
  module_name: packaging
  pypi-package: packaging

test:
  environment: {}
  pipeline:
    - name: Import Test
      uses: python/import
      with:
        import: ${{vars.module_name}}

@justinvreeland justinvreeland marked this pull request as ready for review September 4, 2024 01:02
pnasrat
pnasrat previously approved these changes Sep 4, 2024
Copy link
Contributor

@pnasrat pnasrat left a comment

Choose a reason for hiding this comment

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

LGTM

I would also want @jonjohnsonjr to review and approve

@pnasrat pnasrat dismissed their stale review October 4, 2024 13:47

needs work

@smoser
Copy link
Contributor

smoser commented Oct 11, 2024

This is way helpful. some things I noticed yesterday that I'd like to have (one from justin)

  1. it'd be nice to have the build-deps figured out (adding hatchling or what not when necessary)
  2. at this point we should use either py3-supported-build-base or py3-supported-build-base-dev packages in environment.contents.packages
  3. for things with a '-bin' we need the "move" and '-bin' subpackage. not sure how to do that
  4. add python 3.13 at this point.
  5. I had not seen your 'module_name' and did loads of multi-version with 'import' as the name. yours is probably better, but if we do that we should go and sweep replace.

@smoser
Copy link
Contributor

smoser commented Oct 11, 2024

  1. it'd be nice to have the build-deps figured out (adding hatchling or what not when necessary)

@pnasrat pointed out that this is probably more work than it is worth at this point.

@justinvreeland
Copy link
Contributor Author

justinvreeland commented Oct 11, 2024

  • for things with a '-bin' we need the "move" and '-bin' subpackage. not sure how to do that

Initially I thought maybe we'd have to build it to do this. But we could probably download the actual artifact and tar tf it to see if it generates a bin.

at this point we should use either py3-supported-build-base or py3-supported-build-base-dev packages in environment.contents.packages

This is super easy I can add that no problem

add python 3.13 at this point.

ditto

@smoser smoser enabled auto-merge October 11, 2024 20:04
@justinvreeland justinvreeland force-pushed the jvreeland/update-melange-conversion-script branch from 96cbb22 to c3f5b4b Compare October 11, 2024 22:40
@justinvreeland justinvreeland marked this pull request as draft October 25, 2024 19:00
auto-merge was automatically disabled October 25, 2024 19:00

Pull request was converted to draft

@pnasrat
Copy link
Contributor

pnasrat commented Nov 14, 2024

@justinvreeland I just fixed up the merge conflict to trigger another build and looks like it is passing CI. Is there anything blocking this moving to review?

@justinvreeland justinvreeland marked this pull request as ready for review November 14, 2024 18:37
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.

4 participants