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

packaging: create the plugin-shellcheck-core subpackage #185

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

kdudka
Copy link
Member

@kdudka kdudka commented Sep 20, 2024

... so that one can easily install the run-shellcheck.sh script without unnecessary dependencies on mock etc.

Related: https://issues.redhat.com/browse/OSH-738

@kdudka kdudka requested a review from rhyw September 20, 2024 13:58
@kdudka kdudka self-assigned this Sep 20, 2024
Instead of using `%package -n csmock-common`, we can simply write
`%package common` etc.  This is easier to read and easier to maintain.

Related: csutils#185
kdudka added a commit to kdudka/csmock that referenced this pull request Sep 20, 2024
... so that one can easily install the `run-shellcheck.sh` script
without unnecessary dependencies on `mock` etc.

Related: https://issues.redhat.com/browse/OSH-738
Closes: csutils#185
Copy link
Contributor

@rhyw rhyw left a comment

Choose a reason for hiding this comment

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

I tried on a rhel9 server, the csmock-plugin-shellcheck-core can be installed without bringing additional deps. here's the files installed:

[root@2006276284f1 src]# rpm -ql csmock-plugin-shellcheck-core
/usr/share/csmock
/usr/share/csmock/scripts
/usr/share/csmock/scripts/run-shellcheck.sh
/usr/share/licenses/csmock-plugin-shellcheck-core
/usr/share/licenses/csmock-plugin-shellcheck-core/COPYING

I also installed the csmock-plugin-shellcheck with rpm -i --nodeps:

[root@2006276284f1 src]# rpm -ql csmock-plugin-shellcheck
/usr/lib/python3.9/site-packages/csmock/plugins/__pycache__/shellcheck.cpython-39.opt-1.pyc
/usr/lib/python3.9/site-packages/csmock/plugins/__pycache__/shellcheck.cpython-39.pyc
/usr/lib/python3.9/site-packages/csmock/plugins/shellcheck.py

Comparing with the previous csmock-plugin-shellcheck-3.7.0-1.el9.noarch.rpm package:

[root@8217478ad3a7 src]# rpm -ql csmock-plugin-shellcheck-3.7.0-1.el9
/usr/lib/python3.9/site-packages/csmock/plugins/__pycache__/shellcheck.cpython-39.opt-1.pyc
/usr/lib/python3.9/site-packages/csmock/plugins/__pycache__/shellcheck.cpython-39.pyc
/usr/lib/python3.9/site-packages/csmock/plugins/shellcheck.py
/usr/share/csmock/scripts/run-shellcheck.sh

I can see the difference is that /usr/share/licenses/csmock-plugin-shellcheck-core/COPYING is installed along with the -core package. Assuming it's the intended change, this looks good to me.

I tried later by running the script, /usr/share/csmock/scripts/run-shellcheck.sh * or /usr/share/csmock/scripts/run-shellcheck.sh . all gives desired output to shellcheck-results directory.

Note that since it's not running in a chroot environment, we'll need to install shellcheck for the script to run correctly. Otherwise:

+ timeout 30 shellcheck --format=json test/script/e2e_run.sh
timeout: failed to run command 'shellcheck': No such file or directory
+ timeout 30 shellcheck --format=json test/script/ocm.sh
+ timeout 30 shellcheck --format=json test/script/e2e_setup.sh
timeout: failed to run command 'shellcheck': No such file or directory
timeout: failed to run command 'shellcheck': No such file or directory
timeout: failed to run command 'shellcheck': No such file or directory
+ timeout 30 shellcheck --format=json test/script/util.sh
timeout: failed to run command 'shellcheck': No such file or directory

I think it's also the desired behavior per our discussion in slack.

LGTM

kdudka added a commit to kdudka/csmock that referenced this pull request Sep 20, 2024
... so that one can easily install the `run-shellcheck.sh` script
without unnecessary dependencies on `mock` etc.

Related: https://issues.redhat.com/browse/OSH-738
Related: csutils#185
kdudka added a commit to kdudka/csmock that referenced this pull request Sep 20, 2024
... so that `run-shellcheck.sh` works out of the box.

Related: https://issues.redhat.com/browse/OSH-738
Closes: csutils#185
@kdudka
Copy link
Member Author

kdudka commented Sep 20, 2024

@rhyw Thanks for review!

I can see the difference is that /usr/share/licenses/csmock-plugin-shellcheck-core/COPYING is installed along with the -core package. Assuming it's the intended change, this looks good to me.

IIRC, there is a policy in Fedora packaging that each installed package should provide the license file. csmock-plugin-shellcheck depends on csmock-common, which installs the file. However csmock-plugin-shellcheck-core does not have such a dependency so it installs its own copy of the license file.

Note that since it's not running in a chroot environment, we'll need to install shellcheck for the script to run correctly.

I have introduced a weak dependency on ShellCheck in 3d65f58 to make it work out of the box. The overhead for users of csmock -t shellcheck should be minimal and it can be avoided by disabling installation of weak dependencies if needed.

@rhyw
Copy link
Contributor

rhyw commented Sep 20, 2024

change 3d65f58 looks good. Thanks for the update!

... so that one can easily install the `run-shellcheck.sh` script
without unnecessary dependencies on `mock` etc.

Related: https://issues.redhat.com/browse/OSH-738
Related: csutils#185
... so that `run-shellcheck.sh` works out of the box.

Related: https://issues.redhat.com/browse/OSH-738
Closes: csutils#185
@kdudka
Copy link
Member Author

kdudka commented Sep 20, 2024

@rhyw Thanks for review!

@kdudka kdudka merged commit 57add2e into csutils:main Sep 20, 2024
3 of 39 checks passed
@kdudka kdudka deleted the run-shellcheck branch September 20, 2024 16:09
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