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

CMake: avoid using bash and output redirection #42

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

axel-h
Copy link
Member

@axel-h axel-h commented May 5, 2023

Simplify command and avoids depending on shell features, just use program features.

@axel-h
Copy link
Member Author

axel-h commented Jan 23, 2024

Can we merge this?

Comment on lines -33 to +34
"which dtc && dtc -I dts -O dtb ${CAMKES_ARM_LINUX_DIR}/${device_tree_src} > linux/linux-dtb"
dtc -I dts -O dtb -o linux/linux-dtb ${CAMKES_ARM_LINUX_DIR}/${device_tree_src}
Copy link
Member

Choose a reason for hiding this comment

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

Does anyone know why the which was in there? Is there a danger that there might be multiple dtc commands on the same system and we can get the wrong one?

Copy link
Member Author

Choose a reason for hiding this comment

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

My guess is, it is a smart trick to fail if the dtc program is missing. Because the output redirectection may just produce an empty file, but no error in this case.

Copy link
Member

Choose a reason for hiding this comment

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

Right that makes sense. And we would now also fail, so the behaviour is preserved. Happy with that.

Copy link
Member

@lsf37 lsf37 left a comment

Choose a reason for hiding this comment

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

generally looks fine, I'm just not entirely sure why which was there in the first place

@lsf37
Copy link
Member

lsf37 commented Jan 23, 2024

Looks like Kent introduced it in commit fa4fadf in 2017 in file components/VM/VM.mk. @kent-mcleod surely you remember why you did that 7 years ago? :-)

Do no use shell features, just use program features

Signed-off-by: Axel Heider <[email protected]>
@axel-h
Copy link
Member Author

axel-h commented Jan 24, 2024

Since #46 from @chrisguikema got merged yesterday, I am applying the change there also.

@lsf37 lsf37 merged commit 7dae5f3 into seL4:master Jan 24, 2024
10 checks passed
@axel-h axel-h deleted the patch-axel-8 branch January 24, 2024 09:30
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