-
Notifications
You must be signed in to change notification settings - Fork 90
Start experimental simulators feature #709
Changes from all commits
b5aeb88
ecc9539
6505213
21bd4b1
d496b03
a30f93a
d35a600
3e5d1d9
7966708
df40286
890dcc9
e90a971
5ee2983
16c8e1b
74d1f8a
4af61f9
3be6249
a48ad55
1e9adcd
f659395
b7f7941
d6e7b12
5fd4e79
79c9003
ca5a533
d376c3b
438d72c
76f78c5
2f3bde5
f4b04ca
22a7c55
59e0bf3
9bb6b31
22cfe21
0da2d82
500c191
3a5e87f
f960430
49c52cf
b273d18
06fb952
90d7330
e9e9522
c3dd0f7
522ccea
e806e1a
b1d254e
ebd552a
f32af17
b7b8c96
2944209
fa18ebb
5cf54ea
d83e3e6
9b17072
96cff45
dec89a1
fb90746
37ed0c1
0820d75
0f75d25
467d160
8f47f94
044f7ba
5e2ec23
245b444
d6f4a7e
d741380
22132e0
4109e11
26c4a40
fa7d2ca
bf2147c
b12a34f
434cbb4
f404293
33b8732
5a44d30
37218c1
37756f7
c373fab
6c82ec6
ec8c4e5
a44a135
d91b7ae
62bd05d
ebfd6e7
9cb438e
2f2b5e9
8315961
3906a49
7943335
db0ab9e
2035d67
4b37b2a
527ea8e
cb79239
335d9a0
0b279fd
d259969
bddb295
e7dda55
e057dfa
a4ddaf8
8c02857
7d4246f
d04a0c6
906f9f9
6d338a1
f7e167b
54c37b8
afcdeb9
9306a8c
f4106d6
65aa212
f4c404e
ad6fc22
eb73bf7
e12568e
b144ced
36ebabd
e2efd44
2342e87
ab66721
d53e26c
65b568c
620fa1d
06bead5
72977df
bb09fc4
7b08bff
1042e51
633d88f
b33e9d1
72938ea
c1d7350
dbebdea
623cf46
c7529ca
d9a8222
5a658e2
8376fc3
42c03ef
f3c0382
a80fb6f
b2208ad
394e3a7
7e0f4f3
8f46c55
3db934b
091ace1
bb3c291
939a4b8
54831b9
8847f2a
363b17e
ce0fde5
9433e45
55e0a7c
e50dccf
e4bf294
682d440
069ba53
4c24851
141536a
1b9aa59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # This file configures the pre-commit tool to run an initial | ||
| # suite of lightweight tests on each commit, reducing the | ||
| # probability of failing in CI. | ||
| # For more information on pre-commit, see https://pre-commit.com/. | ||
| repos: | ||
| - repo: https://github.com/doublify/pre-commit-rust | ||
| rev: v1.0 | ||
| hooks: | ||
| - id: fmt | ||
| args: ['--manifest-path', 'src/Simulation/qdk_sim_rs/Cargo.toml', '--'] | ||
| - id: cargo-check | ||
| args: ['--manifest-path', 'src/Simulation/qdk_sim_rs/Cargo.toml', '--'] | ||
| # This step runs cargo-clippy, a linting tool provided with the | ||
| # Rust toolchain. Please see https://github.com/rust-lang/rust-clippy | ||
| # and https://rust-lang.github.io/rust-clippy/master/index.html | ||
| # for more information. | ||
| - id: clippy | ||
cgranade marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| args: ['--manifest-path', 'src/Simulation/qdk_sim_rs/Cargo.toml', '--'] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,33 @@ if ($Env:ENABLE_QIRRUNTIME -ne "false") { | |
| Write-Host "Skipping build of qir runtime because ENABLE_QIRRUNTIME variable is set to: $Env:ENABLE_QIRRUNTIME" | ||
| } | ||
|
|
||
| if ($Env:ENABLE_EXPERIMENTALSIM -ne "false") { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While the feature is still experimental (or at least until we have a good automatic install story for rustup), should this part of the build also get skipped if cargo is not found in the path? That would bridge the gap for a bit for local build while some folks' environments don't yet have Rust. Something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Broke it up a little bit more than your suggestion to make it obvious to the user what they need to do to proceed, and to make sure that we don't have surprising CI failures if prereq installation breaks, but I think this should bridge the gap that you point out. Thanks for flagging that! |
||
| if (-not (Get-Command cargo -ErrorAction SilentlyContinue)) { | ||
| # Cargo was missing, so cannot build experimental simulators. | ||
| # That's fine if running locally, we'll warn the user and then skip. | ||
| # On CI, though, we should fail when the experimental simulator build | ||
| # is turned on by ENABLE_EXPERIMENTALSIM, but we can't actually | ||
| # proceed. | ||
| if ("$Env:TF_BUILD" -ne "" -or "$Env:CI" -eq "true") { | ||
| Write-Host "##[error]Experimental simulators enabled, but cargo was not installed in CI pipeline."; | ||
| } else { | ||
| Write-Warning ` | ||
| "Experimental simulators enabled, but cargo missing. " + ` | ||
| "Either install cargo, or set `$Env:ENABLE_EXPERIMENTALSIM " + ` | ||
| "to `"false`". Skipping experimental simulators."; | ||
| } | ||
| } else { | ||
| # Prerequisites are met, so let's go. | ||
| $expSim = (Join-Path $PSScriptRoot "../src/Simulation/qdk_sim_rs") | ||
| & "$expSim/build-qdk-sim-rs.ps1" | ||
| if ($LastExitCode -ne 0) { | ||
| $script:all_ok = $False | ||
| } | ||
| } | ||
| } else { | ||
| Write-Host "Skipping build of experimental simulators because ENABLE_OPENSIM variable is set to: $Env:ENABLE_OPENSIM." | ||
| } | ||
|
|
||
| function Build-One { | ||
| param( | ||
| [string]$action, | ||
|
|
||
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.
fyi: left a bunch of general items on my review comment for you to consider. thanks!