openSUSE / openSUSE-release-tools

Tools to aid in staging and release work for openSUSE/SUSE
GNU General Public License v2.0
61 stars 97 forks source link

pkglistgen: cli: Explicitly enable product-composer instead of detecting directories. #3091

Closed gleidi-suse closed 3 months ago

gleidi-suse commented 4 months ago

This PR is based on https://github.com/openSUSE/openSUSE-release-tools/pull/3089

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 0% with 54 lines in your changes missing coverage. Please review.

Project coverage is 27.98%. Comparing base (408936d) to head (6d3b8a8). Report is 7 commits behind head on master.

Files Patch % Lines
pkglistgen/tool.py 0.00% 43 Missing :warning:
pkglistgen/cli.py 0.00% 6 Missing :warning:
pkglistgen/engine.py 0.00% 5 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3091 +/- ## ========================================== - Coverage 28.01% 27.98% -0.03% ========================================== Files 86 87 +1 Lines 14987 15003 +16 ========================================== Hits 4199 4199 - Misses 10788 10804 +16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Vogtinator commented 4 months ago

Why?

gleidi-suse commented 4 months ago

Why?

pkglistgen as it is on master is broken on product-composer based products (SL Micro 6.0 and 6.1 currently). Running

./pkglistgen.py --dry   --verbose -A https://api.suse.de/ update_and_solve --project SUSE:SLFO:Products:SL-Micro:6.1 --scope target --git-url https://src.suse.de/products/SL-Micro#6.1 --force

yields

Cloning into '/home/giacomo/.cache/openSUSE-release-tools/pkglistgen/api.suse.de/SUSE:SLFO:Products:SL-Micro:6.1'...
remote: Enumerating objects: 120, done.
remote: Counting objects: 100% (120/120), done.
remote: Compressing objects: 100% (118/118), done.
remote: Total 120 (delta 41), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (120/120), 300.19 KiB | 2.13 MiB/s, done.
Resolving deltas: 100% (41/41), done.
Traceback (most recent call last):
  File "/home/giacomo/code/program-management/openSUSE-release-tools/./pkglistgen.py", line 8, in <module>
    sys.exit(app.main())
             ^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/cmdln.py", line 261, in main
    return self.cmd(args)
           ^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/cmdln.py", line 284, in cmd
    retval = self.onecmd(argv)
             ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/cmdln.py", line 422, in onecmd
    return self._dispatch_cmd(handler, argv)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/cmdln.py", line 1123, in _dispatch_cmd
    return handler(argv[0], opts, *args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/giacomo/code/program-management/openSUSE-release-tools/pkglistgen/cli.py", line 117, in do_update_and_solve
    return solve_project(target_project, scope)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/giacomo/code/program-management/openSUSE-release-tools/pkglistgen/cli.py", line 99, in solve_project
    return self.tool.update_and_solve_target(api, target_project, target_config, main_repo,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/giacomo/code/program-management/openSUSE-release-tools/pkglistgen/tool.py", line 766, in update_and_solve_target
    file_utils.unlink_all_except(product_dir)
  File "/home/giacomo/code/program-management/openSUSE-release-tools/pkglistgen/file_utils.py", line 32, in unlink_all_except
    name_path = os.path.join(path, name)
                ^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen posixpath>", line 76, in join
TypeError: expected str, bytes or os.PathLike object, not NoneType

This is because update_and_solve_target tries to handle every kind of product (both legacy product builder / converter and current product composer) with a single code path. There should probably be different code paths implementing the same interface to handle different kind of products.

This PR is not final and just an attempt at having correctly generated pkglists on SL Micro 6.1 . If it works we can work on a more structured proposal. What do you think?

Vogtinator commented 4 months ago

This PR is not final and just an attempt at having correctly generated pkglists on SL Micro 6.1 . If it works we can work on a more structured proposal. What do you think?

Sure. For now just this particular failure should be guarded IMO

g7 commented 4 months ago

I've tested this in one of the SLE 15 SP6 stagings, old behaviour ("legacy engine") works as expected.

gleidi-suse commented 4 months ago

I know I said I'd work on a more structured proposal but we need this running on SLFO stagings ASAP.

In a perfect universe we should really get rid of all if git_url and if use_product_compose by having different objects encapsulating git/svn and product-builder/product-composer operations exposing the same interface . In any case it is not required to deliver so I'll do that in the future, as they can be implemented in a backward compatible way now that we have --engine .

What do you think?

Vogtinator commented 4 months ago

If you need a quick fix for quick deployment, I'd prefer a as small as possible fix to address only the immediate exception.

Bigger changes like this need a more in-depth review...

gleidi-suse commented 4 months ago

@Vogtinator Could you please give an in depth review to this PR? It was tested on SL Micro 6.1, SLFO Stagings and SLCS 6.0 and it is currently running for SLE 15 SP6 's target. Thank you

Vogtinator commented 4 months ago

@Vogtinator Could you please give an in depth review to this PR? It was tested on SL Micro 6.1, SLFO Stagings and SLCS 6.0 and it is currently running for SLE 15 SP6 's target. Thank you

Can do, but earliest tomorrow

g7 commented 4 months ago

Tested on SLE15SP6 target (sorry, it took a rather long time), and the output is as expected.

gleidi-suse commented 3 months ago

@Vogtinator were you able to take a look at this PR? Thank you