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.

Sign up for GitHub

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

Jump to bottom

Remove installation through meson #19

Merged
merged 1 commit into from
Feb 4, 2025
Merged

Remove installation through meson #19

merged 1 commit into from
Feb 4, 2025

Conversation

sarroutbi
Copy link
Contributor

No description provided.

@sarroutbi sarroutbi force-pushed the 202410111201-install-clevis-from-packages branch 3 times, most recently from 769e4cd to 2e375f1 Compare October 11, 2024 12:16
@sarroutbi
Copy link
Contributor Author

Only merge once clevis-pin-pkcs11 has been released

@sarroutbi sarroutbi changed the title Remove installation through meson [DONOTMERGE] Remove installation through meson Oct 11, 2024
@sarroutbi sarroutbi changed the title [DONOTMERGE] Remove installation through meson Remove installation through meson Nov 8, 2024
@sarroutbi sarroutbi marked this pull request as ready for review November 8, 2024 18:22
@sarroutbi sarroutbi requested a review from kokesak November 8, 2024 18:22
@sarroutbi
Copy link
Contributor Author

clevis-pin-pkcs11 is available in Fedora 40+, RHEL9.6 and RHEL10. We should be good to go with this change

@sarroutbi sarroutbi requested a review from Koncpa November 8, 2024 18:23
kokesak
kokesak reviewed Nov 11, 2024
View reviewed changes
TestHelpers/utils.sh Outdated Show resolved Hide resolved
@kokesak kokesak mentioned this pull request Nov 29, 2024
Merged
@sarroutbi sarroutbi force-pushed the 202410111201-install-clevis-from-packages branch 2 times, most recently from 62e14ed to 6ae33fc Compare January 31, 2025 12:38
@sarroutbi sarroutbi requested review from kokesak and prikryla and removed request for Koncpa January 31, 2025 12:38
prikryla
prikryla reviewed Jan 31, 2025
View reviewed changes
Copy link
Contributor

@prikryla prikryla left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! @kokesak any comments there?

@kokesak
Copy link
Collaborator

kokesak commented Jan 31, 2025 •
edited
Loading

LGTM! @kokesak any comments there?

Looks good. Me personally, I would move the install_clevis_pkcs11 before this assert in the tests

clevis-tests/Sanity/pkcs11/basic/runtest.sh

Line 37 in 6ae33fc

rlAssertRpm $PACKAGE
It can fail, and the installation may never happen.

Also please bear in mind that the clevis package is also being requested by tmt here.

But you can merge as you want guys :D

@sarroutbi sarroutbi force-pushed the 202410111201-install-clevis-from-packages branch from 6ae33fc to 6d62c5b Compare January 31, 2025 14:26
@sarroutbi
Copy link
Contributor Author

LGTM! @kokesak any comments there?

Looks good. Me personally, I would move the install_clevis_pkcs11 before this assert in the tests

clevis-tests/Sanity/pkcs11/basic/runtest.sh

Line 37 in 6ae33fc

rlAssertRpm $PACKAGE

It can fail, and the installation may never happen.

Also please bear in mind that the clevis package is also being requested by tmt here.

But you can merge as you want guys :D

You are completely right. Indeed, clevis-pin-pkcs11 has the rest of the packages as dependency, so not required. I will change the installation previous to the package check.

Thanks @kokesak !!!

@sarroutbi sarroutbi force-pushed the 202410111201-install-clevis-from-packages branch 2 times, most recently from 557bbf3 to 49f11ad Compare January 31, 2025 14:40
kokesak
kokesak approved these changes Jan 31, 2025
View reviewed changes
Copy link
Collaborator

@kokesak kokesak left a comment

Choose a reason for hiding this comment

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

Amazing work, thanks both of you!

@sarroutbi
Remove installation through meson
fdcb68b 
Signed-off-by: Sergio Arroutbi <[email protected]>
@sarroutbi sarroutbi force-pushed the 202410111201-install-clevis-from-packages branch from 49f11ad to fdcb68b Compare January 31, 2025 16:26
@sarroutbi
Copy link
Contributor Author

Error is unrelated to this change, as it can be observed in #28 . I will merge it and open a new PR for the pending issue

@sarroutbi sarroutbi merged commit 6e1549c into RedHat-SP-Security:master Feb 4, 2025
5 of 6 checks passed
@sarroutbi sarroutbi deleted the 202410111201-install-clevis-from-packages branch February 4, 2025 09:03
@sarroutbi sarroutbi mentioned this pull request Feb 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Koncpa Koncpa Koncpa left review comments

@prikryla prikryla prikryla left review comments

@kokesak kokesak kokesak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sarroutbi @kokesak @Koncpa @prikryla