Skip to content

Base.runtests: correctly forward the --sysimage-native-code=no flag if it is provided#42173

Closed
DilumAluthge wants to merge 1 commit intomasterfrom
dpa/runtests-sysimage-native-code
Closed

Base.runtests: correctly forward the --sysimage-native-code=no flag if it is provided#42173
DilumAluthge wants to merge 1 commit intomasterfrom
dpa/runtests-sysimage-native-code

Conversation

@DilumAluthge
Copy link
Copy Markdown
Member

@DilumAluthge DilumAluthge commented Sep 9, 2021

@DilumAluthge DilumAluthge added testsystem The unit testing framework and Test stdlib ci Continuous integration labels Sep 9, 2021
@DilumAluthge DilumAluthge marked this pull request as ready for review September 9, 2021 01:28
@DilumAluthge DilumAluthge force-pushed the dpa/runtests-sysimage-native-code branch from ae8de72 to 98d60f9 Compare September 9, 2021 01:37
run(setenv(`$(julia_cmd()) $(joinpath(Sys.BINDIR::String,
cmd = Base.julia_cmd()
if Base.JLOptions().use_sysimage_native_code == 0
push!(cmd.exec, "--sysimage-native-code=no")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO this should be a part of Base.julia_cmd().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I mean that we should do this check inside of Base.julia_cmd() and push the arguments onto that cmd object, instead of only doing it here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@DilumAluthge
Copy link
Copy Markdown
Member Author

Closing in favor of #42185

@DilumAluthge DilumAluthge deleted the dpa/runtests-sysimage-native-code branch September 9, 2021 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continuous integration testsystem The unit testing framework and Test stdlib

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants