Get the build artifacts together

[kuzminrobin]

Problem description
Currently upon build (pwsh build-qir-runtime.ps1) the build files are generated in the multiple folders inside of QirRuntime folder. In particular:

# (1)
build\
# (2)
samples\StandaloneInputReference\qsharp\{bin, obj, qir}
samples\StandaloneInputReference\qir-standalone-input-reference.ll
# (3)
test\{FullstateSimulator, QIR-dynamic, QIR-static, QIR-tracer}\qsharp\{bin, obj, qir}
# (4)
test\FullstateSimulator\qir-test-simulator.ll
test\QIR-dynamic\qir-test-random.ll
test\QIR-static\qir-gen.ll
test\QIR-tracer\tracer-qir.ll

This has a few disadvantages:

• If somebody needs to rebuild all (e.g. they updated some of the files), they need to remove all those built folders/files ((1)..(4)). If they just remove build\ then the .ll files will not be regenerated upon local build (pwsh build-qir-runtime.ps1), showing the confusing errors.
• The documentation/instructions about how to rebuild all, have to be more complicated than could be (which will likely make the reader frown :-\ ).

Would be nice if all the build files were generated in a single directory, e.g. build\{Debug | Release}.

Consider making the following changes.

• Change the {test, samples}\**\*.csproj files such that they generate the build files inside of build\{Debug | Release}.
• Change the build-qir-runtime.ps1 such that in the end of function Build-QirProject (the first of the two Copy-Item commands) it either does not copy the .ll files to {test, samples} (see (4) above) or copies within build\{Debug | Release}.
• Update README.md with the instructions about how to rebuild all (by removing the build\ folder).

---

Optionally consider the following

[8:27 AM] Robin Kuzmin
Another thing is worth mentioning is that the .ll files are regenerated from .qs, only if .ll is absent.
If the .qs is newer than .ll, the .ll is not re-generated. Looks like a certain dependency is missing in the CMake files or .ps1 files.

[8:35 AM] Stefan Wernli
Yes, that’s correct, and intentional. The pipeline gets very mad when the script tries to build the individual csproj files, which is why I added them to the solution. The expected workflow is to use the QIR scripts to build the QIR runtime, and if any changes to .qs files has been made to rebuild those using dotnet build on the Simulation.sln
​[8:35 AM] Stefan Wernli
Yet another thing it would be good to clarify in the readme, now that I’m thinking of it

​[10:34 AM] Robin Kuzmin
Stefan, I assume you meant the following:
"The pipeline gets very mad when the script (e.g. qsharp-runtime\src\QirRuntime\build-qir-runtime.ps1 ) tries to build the individual csproj files (e.g. qsharp-runtime\src\QirRuntime\test\QIR-static\qsharp\qir-gen.csproj ), which is why I added them to the solution (qsharp-runtime\Simulation.sln)".
As I now understand, the dependency that causes the regeneration of the .ll files upon the update of the .qs files, is observed in (or is built into) the Simulation.sln.
If that's the way such dependencies work, then may be we can create one more .sln somewhere inside of qsharp-runtime\src\QirRuntime and add the .csproj files (e.g. test\QIR-static\qsharp\qir-gen.csproj) to that .sln too? As a result, the .csproj will be included into two solutions - Simualtion.sln and QirRuntime local .sln. And the .ll files will be rebuilt upon update to the .qs, both when building QirRuntime only, and when building the whole repo qsharp-runtime. (Just an idea to consider. I'm not sure if this idea works or is needed at all)

Edited​[10:39 AM] Stefan Wernli
that's an interesting idea, yeah. The second solution could be for local development only, such that the pipeline doesn't build it at all and only relies on the Simulation.sln. There is a bit of extra overhead in keeping the two solutions in sync, which we should take into account; eg: a new qir test project is created and added to the second solution but not the first. In that case the pipeline will fail, which should prevent anything like that from getting checked in.
​[10:41 AM] Stefan Wernli
for me personally, I try to keep my local development flow to be as similar to what runs in the pipeline as possible that way I get fewer surprises when submitting PRs, so I try to always use the same scripts the pipeline uses and not introduce too much local-dev-specific tooling into the repo. But not everyone likes to work that way. There was a long thread on the C&R channel somewhere regarding the disconnect between build scripts the pipeline uses and what people do locally on their machines, and opinions on the team vary.
​[10:45 AM] Robin Kuzmin
Very good points. I totally agree.
Curious, can Simulation.sln
instead of directly including the .csproj files (like src\QirRuntime\test\QIR-static\qsharp\qir-gen.csproj)
include another .sln (QirRuntime local .sln, that includes the QirRuntime local .csproj files)?

​[10:51 AM] Cesar Zaragoza Cortes
Another option that we could do for local development is to add a generateQirAlways parameter to the script which is false by default. I've been doing something similar and it has been convenient.

[swernli]

For me personally, I prefer the existing default behavior we use throughout our repos where a csproj/fsproj always produces bin and obj folders as peers. Collecting all the build output in one place doesn't seem that useful to me, but it's possible I'm just very accustomed to the current set up.

[cgranade]

I think it would be highly surprising to users of the dotnet command line tool coming to our repo for the first time to find intermediate build files and build artifacts anywhere but in bin and obj folders adjacent to each *.csproj file. Similarly, the Python, node.js, and Rust build toolchains each by default collect their intermediate files in folders adjacent to the corresponding build definitions. That is, for the majority of our build languages (C#, Python, JavaScript / TypeScript), having build output peered to build definitions meets the default behavior that new contributors will likely expect, as to @swernli's point.

[kuzminrobin]

OK, thank you for the feedback, @swernli, @cgranade.
You convinced me. It makes sense to leave the current implementation as is. We can resolve this issue with conclusion "By Design".

[kuzminrobin]

The current implementation is "By Design".
For consistency with the other toolchains it makes sense to leave the implementation as is.