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

platforms: allow setting extra flags to be passed to build #310

Merged
merged 3 commits into from
Jan 30, 2024
Merged

platforms: allow setting extra flags to be passed to build #310

merged 3 commits into from
Jan 30, 2024

Conversation

Ivan-Velickovic
Copy link
Contributor

Untested, not sure how to test (I guess its Python so you never know until it is too late :) )

Ivan-Velickovic added a commit that referenced this pull request Jan 19, 2024
@Ivan-Velickovic
platforms: disable MaaXBoard temporarily
d5eef20 
Until #310 is merged.

Signed-off-by: Ivan Velickovic <[email protected]>
@Ivan-Velickovic Ivan-Velickovic mentioned this pull request Jan 19, 2024
Merged
lsf37 pushed a commit that referenced this pull request Jan 19, 2024
@Ivan-Velickovic
platforms: disable MaaXBoard temporarily (#311)
08c98dc 
Until #310 is merged.

Signed-off-by: Ivan Velickovic <[email protected]>
axel-h
axel-h reviewed Jan 19, 2024
View reviewed changes
seL4-platforms/platforms.py Outdated Show resolved Hide resolved
axel-h
axel-h approved these changes Jan 29, 2024
View reviewed changes
Copy link
Member

@axel-h axel-h left a comment

Choose a reason for hiding this comment

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

Looks good to me.

lsf37
lsf37 approved these changes Jan 30, 2024
View reviewed changes
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.

Cool. I like it with reusing settings.

@lsf37 lsf37 added the boards related to test boards and machines label Jan 30, 2024
@Ivan-Velickovic Ivan-Velickovic merged commit 59703f6 into master Jan 30, 2024
7 checks passed
@Ivan-Velickovic Ivan-Velickovic deleted the sel4test_flags branch January 30, 2024 03:47
@Ivan-Velickovic
Copy link
Contributor Author

I gave this a test to make sure sel4test ran after merging and to my surprise, adding Sel4testAllowSettingsOverride actually does more than I expected.

For example I ran sel4test with ../init-build.sh -DPLATFORM=zynqmp -DSel4testAllowSettingsOverride=1 and ./init-build.sh -DPLATFORM=zynqmp which I thought would produced the exact same image since no settings are actually being overridden, but it looks like for some reason that the addition of Sel4testAllowSettingsOverride now causes sel4test to default to release mode. Not sure if this will be a problem with CI if debug mode is explicitly being turned on.

@lsf37
Copy link
Member

lsf37 commented Jan 30, 2024

Hm, it looks like that might influence a whole bunch of settings:
https://github.com/seL4/sel4test/blob/0a488b92bc7a08acc14d49524d1f22656f286627/settings.cmake#L43-L134

That setting is pretty misnamed, it's not an override, but prevents any default settings from applying. This is probably not really what we want to use. We might need to add an actual override that allows you to set things in addition to the defaults.

Ivan-Velickovic added a commit that referenced this pull request Jan 30, 2024
@Ivan-Velickovic
platforms: revert custom settings
81ba93c 
Reverts some of the changes in
#310 that lead to
unexpected side-affects. The 'Sel4testAllowSettingsOverride'
prevents any default settings being applied in addition to
allowing the settings to be overriden.

Signed-off-by: Ivan Velickovic <[email protected]>
@Ivan-Velickovic Ivan-Velickovic mentioned this pull request Jan 30, 2024
Merged
lsf37 pushed a commit that referenced this pull request Jan 30, 2024
@Ivan-Velickovic @lsf37
platforms: revert custom settings
fcb7f29 
Reverts some of the changes in
#310 that lead to
unexpected side-affects. The 'Sel4testAllowSettingsOverride'
prevents any default settings being applied in addition to
allowing the settings to be overriden.

Signed-off-by: Ivan Velickovic <[email protected]>
@axel-h axel-h mentioned this pull request Jan 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsf37 lsf37 lsf37 approved these changes

@axel-h axel-h axel-h approved these changes

Assignees
No one assigned
Labels
boards related to test boards and machines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ivan-Velickovic @lsf37 @axel-h