-
-
Notifications
You must be signed in to change notification settings - Fork 38
Add Windows release options for JIT builds #241
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
base: master
Are you sure you want to change the base?
Conversation
windows-release/stage-build.yml
Outdated
${{ if ne(parameters.DoJIT, 'true') }}: | ||
ExtraOptions: '' | ||
${{ elseif ne(parameters.DoJITEnabled, 'true') }}: | ||
ExtraOptions: '--experimental-jit-off' | ||
${{ else }}: | ||
ExtraOptions: '--experimental-jit' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love the repetition of this. Two options you can try:
- add an "ExtraBuildOptions" parameter to this template and put it unconditionally in all of these, then have the condition in
azure-pipelines
to choose which of these to pass. - add a
variables
block to each job in this file with anExtraOptions2
, and update the build steps file to use both
Possibly some combination of the two approaches is best. The main thing I want to optimise for is only seeing the two options (i.e. '--experimental-jit'
) once (each), since they're global across the whole matrix.
windows-release/azure-pipelines.yml
Outdated
- name: DoJIT | ||
displayName: "Build the JIT compiler (3.14 and later)" | ||
type: boolean | ||
default: false | ||
- name: DoJITEnabled | ||
displayName: "Enable the JIT compiler by default (not used yet)" | ||
type: boolean | ||
default: false | ||
- name: DoJITFreethreaded | ||
displayName: "Build the JIT compiler for free-threaded builds (not used yet)" | ||
type: boolean | ||
default: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of ordering, use descending version numbers, and put the future proofing ones right at the end. Having the version numbers in there is very important or we'll definitely get the releases wrong :)
If it makes more sense, you can make a drop down with type: string
and values:
like the signing certificates above. Trick here is that you can't select "none" unless it's actually in the list, and you'll need eq()
checks (unless you can figure out a way to use the option directly and have better display text, which may be possible, but I don't know how).
Thanks for the suggestions, it's quite a bit cleaner now. |
I'm not sure how to test this, or even if this is the right approach, so I'd appreciate @zooba's eyes on this. Hopefully I'm on the right track, though.
I went ahead and "future-proofed" this by adding additional options for enabling the JIT by default (not desired until 3.15 at the earliest), and also building it on free-threaded builds (not possible until 3.15 at the earliest). These options can be dropped though, if they add too much noise.