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

Add dev-util/pkgcheck to SDK #2205

Merged
merged 12 commits into from
Sep 4, 2024
Merged

Add dev-util/pkgcheck to SDK #2205

merged 12 commits into from
Sep 4, 2024

Conversation

krnowak
Copy link
Member

@krnowak krnowak commented Aug 6, 2024

It seems to be a useful tool, especially when trying to upstream things to Gentoo, as there is now a requirement to call pkgcheck on the changed packages.

CI: http://jenkins.infra.kinvolk.io:8080/job/container/job/sdk/1704/cldsv/

Blocked on #2193.

  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)
  • Inspected CI output for image differences: /boot and /usr size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.

@tormath1
Copy link
Contributor

tormath1 commented Aug 8, 2024

Before reviewing, I see there is a Docker image: https://github.com/pkgcore/pkgcheck/pkgs/container/pkgcheck did you try it? Something like:

$ podman run --rm -ti -w /opt -v "$PWD:/opt" ghcr.io/pkgcore/pkgcheck:sha-257d394
Python 3.12.3 (main, May 14 2024, 07:34:56) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pkgcheck import scan
>>> for result in scan(['/opt']):
...     print(result)
...
WARNING:pkgcore:'@DESCRIPTION:', line 30: missing args
WARNING:pkgcore:'@FUNCTION::subversion__svn_info', @RETURN or @DESCRIPTION required
WARNING:pkgcore:'@FUNCTION::subversion__get_repository_uri', @RETURN or @DESCRIPTION required
WARNING:pkgcore:'@FUNCTION::subversion__get_wc_path', @RETURN or @DESCRIPTION required
WARNING:pkgcore:'@FUNCTION::subversion__get_peg_revision', @RETURN or @DESCRIPTION required
gentoo -- updating eclass cache: xorg-3
genntoo -- updating git cache: commit date: 2023-07-26
...

You can also override the --entrypoint bash to get a bash interpreter and run the pkgcheck <scan> as usual

Copy link

github-actions bot commented Aug 8, 2024

@krnowak
Copy link
Member Author

krnowak commented Aug 8, 2024

Ha, honestly, not really, I haven't tried using docker image. So not sure, I'm thinking that having it in SDK out-of-the-box still may be useful. It's something I could eventually add to the automation to check ebuilds in overlay.

Copy link
Contributor

@chewi chewi left a comment

Choose a reason for hiding this comment

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

You've changed some packages (pip, u-boot-tools, gflags, glog, tcl...) that aren't needed for pkgcheck, which makes this addition look heavier than it is.

I'm strongly in favour of adding this tool using packages rather than using Docker. It's part of my regular Gentoo workflow, and it's extremely good at catching QA issues. I feel like I'm flying blind without it. Using it with Flatcar from Gentoo itself is a bit of a pain because it's hardcoded to look at /etc/portage, which is pointing to the wrong repositories. I've had to make a wrapper to work around this.

#!/bin/bash

if [[ ${PWD} == ${HOME}/Projects/flatcar/scripts/sdk_container/src/third_party/* ]]; then
        exec bwrap --bind / / --bind ~/etc/portage/repos.conf /etc/portage/repos.conf --dev-bind /dev /dev /usr/bin/pkgcheck "${@}"
else
        exec /usr/bin/pkgcheck "${@}"
fi

@krnowak
Copy link
Member Author

krnowak commented Aug 30, 2024

You've changed some packages (pip, u-boot-tools, gflags, glog, tcl...) that aren't needed for pkgcheck, which makes this addition look heavier than it is.

Oh, I just need to rebase it. I think I based this branch on top of a branch that was adding dev-python/pip to automation, which resulted in bringing in a bunch of packages. Will do it on Monday.

I'm strongly in favour of adding this tool using packages rather than using Docker. It's part of my regular Gentoo workflow, and it's extremely good at catching QA issues. I feel like I'm flying blind without it. Using it with Flatcar from Gentoo itself is a bit of a pain because it's hardcoded to look at /etc/portage, which is pointing to the wrong repositories. I've had to make a wrapper to work around this.

Thanks for the hint. Talking about wrappers: do you think that having a wrapper inside SDK would be useful? I mean wrappers like pkgcheck-amd64-usr or pkgcheck-arm64-usr, that would use profiles from /build/{amd,arm}64-usr? Would there be something in pkgcheck that could take advantage of checking the ebuild against the generic profiles intead of SDK profiles being in /etc/portage?

Also, would pkgcheck maintainers be open to the idea of specifying a different config root? That way the bubblewrap-like wrappers wouldn't be necessary.

#!/bin/bash

if [[ ${PWD} == ${HOME}/Projects/flatcar/scripts/sdk_container/src/third_party/* ]]; then
        exec bwrap --bind / / --bind ~/etc/portage/repos.conf /etc/portage/repos.conf --dev-bind /dev /dev /usr/bin/pkgcheck "${@}"
else
        exec /usr/bin/pkgcheck "${@}"
fi

@chewi
Copy link
Contributor

chewi commented Aug 30, 2024

Thanks for the hint. Talking about wrappers: do you think that having a wrapper inside SDK would be useful? I mean wrappers like pkgcheck-amd64-usr or pkgcheck-arm64-usr, that would use profiles from /build/{amd,arm}64-usr? Would there be something in pkgcheck that could take advantage of checking the ebuild against the generic profiles intead of SDK profiles being in /etc/portage?

I don't think that's needed. Although pkgcheck seems to need a profile configured, it shows warnings across different profiles.

Also, would pkgcheck maintainers be open to the idea of specifying a different config root? That way the bubblewrap-like wrappers wouldn't be necessary.

It used to support that, but they dropped that support for some reason.

@krnowak
Copy link
Member Author

krnowak commented Aug 30, 2024

Thanks for the hint. Talking about wrappers: do you think that having a wrapper inside SDK would be useful? I mean wrappers like pkgcheck-amd64-usr or pkgcheck-arm64-usr, that would use profiles from /build/{amd,arm}64-usr? Would there be something in pkgcheck that could take advantage of checking the ebuild against the generic profiles intead of SDK profiles being in /etc/portage?

I don't think that's needed. Although pkgcheck seems to need a profile configured, it shows warnings across different profiles.

Alright.

Also, would pkgcheck maintainers be open to the idea of specifying a different config root? That way the bubblewrap-like wrappers wouldn't be necessary.

It used to support that, but they dropped that support for some reason.

Reason isn't very detailed: pkgcore/pkgcheck@b659846

@krnowak
Copy link
Member Author

krnowak commented Sep 3, 2024

It seems to be unusable by default right now - got some traceback when invoking pkgcheck scan:

Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/pkgcore/config/central.py", line 204, in _instantiate
    self._instance = callable_obj(*pargs, **configdict)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/domain.py", line 293, in __init__
    for k in self.profile.profile_only_variables.intersection(settings.keys()):
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 720, in profile_only_variables
    return frozenset(self.default_env.get("PROFILE_ONLY_VARIABLES", ()))
                     ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/klass.py", line 205, in __get__
    obj = self.function(instance)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 703, in default_env
    d = dict(self.node.default_env.items())
             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/klass.py", line 205, in __get__
    obj = self.function(instance)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 147, in _load_and_invoke
    return func(self, data)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 505, in default_env
    rendered.update(parent.default_env.items())
                    ^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/klass.py", line 205, in __get__
    obj = self.function(instance)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 147, in _load_and_invoke
    return func(self, data)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 504, in default_env
    for parent in self.parents:
                  ^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/klass.py", line 205, in __get__
    obj = self.function(instance)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 276, in parents
    for path, line, lineno in self.parent_paths:
                              ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/klass.py", line 205, in __get__
    obj = self.function(instance)
          ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 147, in _load_and_invoke
    return func(self, data)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/ebuild/profiles.py", line 259, in parent_paths
    abspath(pjoin(location, "profiles", profile_path)),
                  ^^^^^^^^
UnboundLocalError: cannot access local variable 'location' where it is not associated with a value

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/pkgcore/config/central.py", line 130, in instantiate
    self._instance = self._instantiate()
                     ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/config/central.py", line 209, in _instantiate
    raise errors.InstantiationError(
pkgcore.config.errors.InstantiationError: Failed instantiating section 'livefs': exception caught from 'pkgcore.ebuild.domain.domain'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/pkgcore/config/central.py", line 592, in get_default
    return defaults[0][1].instantiate()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/config/central.py", line 134, in instantiate
    raise errors.InstantiationError(self.name) from e
pkgcore.config.errors.InstantiationError: Failed instantiating section 'livefs'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python-exec/python3.11/pkgcheck", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcheck/scripts/__init__.py", line 48, in main
    run(os.path.basename(sys.argv[0]))
  File "/usr/lib/python3.11/site-packages/pkgcheck/scripts/__init__.py", line 40, in run
    sys.exit(tool())
             ^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/tool.py", line 81, in __call__
    ret = self.main()
          ^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcheck/cli.py", line 20, in main
    return super().main()
           ^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/tool.py", line 191, in main
    self.handle_exec_exception(e)
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/tool.py", line 173, in main
    self.options, func = self.parse_args(
                         ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/tool.py", line 128, in parse_args
    self.handle_exec_exception(e)
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/tool.py", line 102, in parse_args
    options = self.parser.parse_args(args=args, namespace=namespace)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/arghparse.py", line 1260, in parse_args
    args, unknown_args = self.parse_known_args(args, namespace)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/arghparse.py", line 1247, in parse_known_args
    namespace, args = self._parse_known_args(args, namespace)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/argparse.py", line 2131, in _parse_known_args
    stop_index = consume_positionals(start_index)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/argparse.py", line 2087, in consume_positionals
    take_action(action, args)
  File "/usr/lib/python3.11/argparse.py", line 1983, in take_action
    action(self, namespace, argument_values, option_string)
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/arghparse.py", line 539, in __call__
    namespace, arg_strings = parser.parse_known_args(arg_strings, namespace)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/arghparse.py", line 1244, in parse_known_args
    namespace, args = functor(parser, namespace, args)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcheck/scripts/pkgcheck_scan.py", line 389, in _setup_scan
    namespace.target_repo = _determine_target_repo(namespace)
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcheck/scripts/pkgcheck_scan.py", line 282, in _determine_target_repo
    for repo in namespace.domain.ebuild_repos_raw:
                ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/arghparse.py", line 589, in __getattribute__
    val(self, name)
  File "/usr/lib/python3.11/site-packages/snakeoil/cli/arghparse.py", line 372, in __call__
    self.invokable(namespace, attr)
  File "/usr/lib/python3.11/site-packages/pkgcore/util/commandline.py", line 227, in store_default
    obj = config.get_default(config_type)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/pkgcore/config/central.py", line 596, in get_default
    raise errors.ConfigurationError(
pkgcore.config.errors.ConfigurationError: failed instantiating default domain 'livefs'

@chewi
Copy link
Contributor

chewi commented Sep 3, 2024

It certainly works for me outside the SDK. I'll give this a try.

@krnowak
Copy link
Member Author

krnowak commented Sep 3, 2024

Just to be clear - that was me trying to run pkgcheck inside the SDK container.

@krnowak
Copy link
Member Author

krnowak commented Sep 3, 2024

For the record, what I did was basically: git checkout main-4078.0.0-pkgcheck and then ./run_sdk_container -t -U. This gives yout eventually a bash prompt inside SDK.

@chewi
Copy link
Contributor

chewi commented Sep 3, 2024

Reproduced. I'll see if I can figure it out.

@chewi
Copy link
Contributor

chewi commented Sep 3, 2024

It works when make.profile points to a profile in portage-stable like /mnt/host/source/src/third_party/portage-stable/profiles/default/linux/amd64/23.0. Perhaps this is a pkgcheck bug where it doesn't like the profile being in an overlay.

@chewi
Copy link
Contributor

chewi commented Sep 3, 2024

Actually, it fails in a similar way when run against packages in coreos-overlay, regardless of the profile. I guess it just doesn't like something about coreos-overlay, but it works outside the SDK. 🤔

@chewi
Copy link
Contributor

chewi commented Sep 3, 2024

Got it! It's pkgcore/pkgcore#435, which I already fixed, but the fix hasn't been released yet. You could patch it in.

It's from Gentoo commit 28c84fdb12e224eb39caff2a9e3f5979ebbeb6a0.
It's from Gentoo commit 0534a4d8d9c7ee83e868051fe33db8d0e88d9ab7.
It's from Gentoo commit 5a6bf87099bc8fb106d17f77fad7b9aef5036325.
It's from Gentoo commit 2b39a8d7500f05c364de764662755cdd58a9918a.
It's from Gentoo commit 7b7fb48115669de91e0e5b0abea5524aa945cd51.
It's from Gentoo commit 79431052dbb681133cb401ceea571c622ebb925d.
It's from Gentoo commit cd426afc7f32ac84206f4156fc8ed0d20bd79246.
@krnowak
Copy link
Member Author

krnowak commented Sep 4, 2024

Seems to be working now. It will spit a lot of warnings about nonsolvable or nondexistent deps for some packages though, because of our repos being a subset of Gentoo.

@chewi
Copy link
Contributor

chewi commented Sep 4, 2024

This might be a good idea:

/etc/pkgcheck/pkgcheck.conf

[DEFAULT]
keywords = -NonexistentDeps,-NonsolvableDepsInDev,-NonsolvableDepsInExp,-NonsolvableDepsInStable

@krnowak
Copy link
Member Author

krnowak commented Sep 4, 2024

This might be a good idea:

/etc/pkgcheck/pkgcheck.conf

[DEFAULT]
keywords = -NonexistentDeps,-NonsolvableDepsInDev,-NonsolvableDepsInExp,-NonsolvableDepsInStable

It is a good idea. Please see the new commit.

Co-authored-by: James Le Cuirot <jlecuirot@microsoft.com>
@krnowak
Copy link
Member Author

krnowak commented Sep 4, 2024

Tested locally, seems to be working.

@krnowak krnowak merged commit 0009a0d into main Sep 4, 2024
1 check failed
@krnowak krnowak deleted the krnowak/pkgcheck branch September 4, 2024 14:45
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.

3 participants