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

Fix build error if INITRAMFS_IMAGE undefined #637

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

quic-vkraleti
Copy link
Contributor

Fixed below error in linux-qcom-uki.bb when INITRAMFS_IMAGE is undefined.

ERROR: Error for linux-qcom-uki.bb:do_configure[depends], dependency '${@ '${INITRAMFS_IMAGE}:do_image_complete' if d.getVar('INITRAMFS_IMAGE') else ''}' does not contain exactly one ':' character. Task 'depends' should be specified in the form 'packagename:task'

Copy link
Collaborator

@lumag lumag left a comment

Choose a reason for hiding this comment

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

Rule of thumb "Also do this and that" => separate commits.

Also please use imperative language to describe the second commit ('remove' instead of 'removed').

@lumag
Copy link
Collaborator

lumag commented Sep 12, 2024

Also could you please expand CI configuration so that such cases can be caught (please submit a PR with CI changes separately so that we can merge it first and then test this PR against the merged tree).

@quic-vkraleti
Copy link
Contributor Author

and CI configuration so that such cases can be caught (please submit a PR with CI changes separately so that we can merge it first and then test this

Sure, I'll update CI scripts in a separate PR and get back.

Reword error message looking for a valid kernel image as
SKIP_RECIPE[linux-qcom-uki] also mentioning the same.

Signed-off-by: Viswanath Kraleti <quic_vkraleti@quicinc.com>
@lumag
Copy link
Collaborator

lumag commented Sep 12, 2024

I don't see CI failing after the CI change in #639 . Was it enough to demonstrate the issue being fixed by this PR?

@quic-vkraleti
Copy link
Contributor Author

I don't see CI failing after the CI change in #639 . Was it enough to demonstrate the issue being fixed by this PR?

To reproduce need to set INITRAMFS_IMAGE = "". Is there any configuration in CI that tests kernel without initrd?

@lumag
Copy link
Collaborator

lumag commented Sep 12, 2024

I don't see CI failing after the CI change in #639 . Was it enough to demonstrate the issue being fixed by this PR?

To reproduce need to set INITRAMFS_IMAGE = "". Is there any configuration in CI that tests kernel without initrd?

I asked you to add necessary CI configuration.

@quic-vkraleti
Copy link
Contributor Author

I don't see CI failing after the CI change in #639 . Was it enough to demonstrate the issue being fixed by this PR?

To reproduce need to set INITRAMFS_IMAGE = "". Is there any configuration in CI that tests kernel without initrd?

I asked you to add necessary CI configuration.

#644 updated build template to build with & without initramfs.

@lumag lumag mentioned this pull request Sep 13, 2024
Copy link
Collaborator

@lumag lumag left a comment

Choose a reason for hiding this comment

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

I've reviewed the error case. Please keep existing do_configure[depends] and provide default value (empty) for the INITRAMFS_IMAGE, like kernel.bbclass does.

@lumag
Copy link
Collaborator

lumag commented Sep 16, 2024

@quic-vkraleti any chance of getting this done? I'd like to merge the fix and then backport necessary changes to the scarthgap branch.

Below error seen when INITRAMFS_IMAGE is undefined. Fix it by defining
INITRAMFS_IMAGE with a weak assignment.

ERROR: Error for linux-qcom-uki.bb:do_configure[depends], dependency
'${@ '${INITRAMFS_IMAGE}:do_image_complete' if d.getVar('INITRAMFS_IMAGE') else ''}' does not contain exactly one ':' character.
Task 'depends' should be specified in the form 'packagename:task'

Signed-off-by: Viswanath Kraleti <quic_vkraleti@quicinc.com>
@quic-vkraleti
Copy link
Contributor Author

@quic-vkraleti any chance of getting this done? I'd like to merge the fix and then backport necessary changes to the scarthgap branch.

Sorry for the delay. Updated as requested. Please check.

@lumag
Copy link
Collaborator

lumag commented Sep 17, 2024

Thank you! LGTM!

@lumag lumag merged commit 4cd1633 into Linaro:master Sep 17, 2024
2 checks passed
@quic-vkraleti quic-vkraleti deleted the uki-changes branch September 19, 2024 08:54
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