Skip to content

Conversation

enriquebelarte
Copy link
Contributor

We were building rpms with the original unsigned kmods

@elenagerman
Copy link
Contributor

Looks perfect, but WDYT about implementing 2-4 tests to verify related code changes?

@enriquebelarte enriquebelarte force-pushed the fix-rpm-signed-kos branch 2 times, most recently from ffa842e to d222862 Compare October 13, 2025 09:09
Comment on lines 121 to 123
# Copy signed kmods and envfile to working directory
cp "${SIGNED_KMODS_PATH}"/*.ko .
cp "${SIGNED_KMODS_PATH}/envfile" .
Copy link
Contributor

Choose a reason for hiding this comment

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

Code and test look solid, thanks for the fix.
Small Suggestions:

Before copying, should we consider checking if the envfile and any .ko files exist, to provide clearer error messages if they are missing?

When using cp "${SIGNED_KMODS_PATH}"/*.ko ., if there are no .ko files, cp will fail. You might want to handle this case gracefully? correct me if I am wrong because I am not the expert on OOT-kmods-rpms :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The checks were moved before the cp command to provide better context in case of failure.
Thanks @happybhati

We were building rpms with the original unsigned kmods

Signed-off-by: Enrique Belarte Luque <[email protected]>
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