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

infra: tests: add rpmlint to existing rpm tests #5929

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

elkoniu
Copy link
Contributor

@elkoniu elkoniu commented Oct 10, 2024

rpmlint is used in Fedora dist git in the release process. Having it included in the upstream CI will allow us early detection of the potential linter issues in spec / rpms.

  • Blocked on fixing the remaining RPM lint issues

@github-actions github-actions bot added infrastructure Changes affecting mostly infrastructure f42 Fedora 42 labels Oct 10, 2024
Makefile.am Outdated Show resolved Hide resolved
@KKoukiou KKoukiou added the blocked Don't merge this pull request! label Oct 10, 2024
Copy link

Infrastructure check failed on these files. Please do a double check of these files before merge!

Makefile.am

Copy link

Infrastructure check failed on these files. Please do a double check of these files before merge!

Makefile.am

Copy link

Infrastructure check failed on these files. Please do a double check of these files before merge!

Makefile.am

@elkoniu
Copy link
Contributor Author

elkoniu commented Oct 13, 2024

@KKoukiou I have update this PR so when rpmlint.toml file will be presented - it will be used by linter. According to documentation this configuration will be merged with default config, which in our case will mean silencing some errors we want to be skipped.

@jkonecny12
Copy link
Member

I really like this idea.

@elkoniu
Copy link
Contributor Author

elkoniu commented Oct 22, 2024

This will not fail after PRs introducing config file will be merged.

Makefile.am Outdated
Comment on lines 336 to 339
dnf install -y /tmp/anaconda/result/build/01-rpm-build/*.rpm; \
cd /tmp/anaconda; \
if [ -f rpmlint.toml ]; then LINT_ARG="--config rpmlint.toml"; else LINT_ARG=""; fi; \
rpmlint *.spec.in result/build/01-rpm-build/*.rpm ${LINT_ARG}'
Copy link
Member

Choose a reason for hiding this comment

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

I think the better would solution would be to have this as script in the container. Let's try to avoid overloading of this command to avoid installation of the package to the host system because of issue in bash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean to have a separated script with the content rpmlint ..... an just call it from here? To be honest the part with if [ -f rpmlint.toml ] can be skipped as I assume the file will be always presented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the code so it just calls rpmlint with the config file and path we use for RPM storage.

Copy link

Infrastructure check failed on these files. Please do a double check of these files before merge!

Makefile.am

@elkoniu
Copy link
Contributor Author

elkoniu commented Oct 22, 2024

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member

jkonecny12 commented Oct 23, 2024

I wonder, do we need to install Anaconda packages? You are already giving RPMs and spec to the linter. Isn't that enough?

@elkoniu
Copy link
Contributor Author

elkoniu commented Oct 23, 2024

I wonder, do we need to install Anaconda packages? You are already giving RPMs and spec to the linter. Isn't that enough?

I think it is happening because I have appended existing rpm-test - which purpose is I guess to test if produced RPMs can be installed. Katerina proposed to put rpmlint test here instead of creating separated make target.

@KKoukiou KKoukiou changed the title infra: Add rpmlint test to upstram CI tests: add rpmlint to existing rpm tests Oct 25, 2024
rpmlint is used in Fedora dist git in the release process.
Having it included in the upstream CI will enable early detection
of the potential linter issues in spec / rpms.
@KKoukiou KKoukiou changed the title tests: add rpmlint to existing rpm tests infra: tests: add rpmlint to existing rpm tests Oct 25, 2024
Copy link

Infrastructure check failed on these files. Please do a double check of these files before merge!

Makefile.am

@KKoukiou
Copy link
Contributor

/kickstart-tests --waive infra only

@KKoukiou KKoukiou merged commit c8fec9e into rhinstaller:master Oct 25, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Don't merge this pull request! f42 Fedora 42 infrastructure Changes affecting mostly infrastructure
Development

Successfully merging this pull request may close these issues.

3 participants