[build-script] Add option --infer to infer dependencies. #32256
Conversation
|
@swift-ci python lint |
gottesmm
force-pushed
the
pr-6a889299976ad0e3dfbc1849e1b7edaf177c53ce
branch
from
1f965d5 to
76ab50f
Compare
|
@swift-ci python lint |
3 similar comments
|
@swift-ci python lint |
|
@swift-ci python lint |
|
@swift-ci python lint |
|
@swift-ci smoke test |
|
In a subsequent commit, I am going to make this just imply install_all since I think that is the typical behavior people will want. |
|
I did a little more work here and I realized I want to do install-all as part of this. Really --infer should always imply --install-all. So once I land a few other things, I am going to push the combined patch to here. Then I should be done. |
gottesmm
force-pushed
the
pr-6a889299976ad0e3dfbc1849e1b7edaf177c53ce
branch
from
76ab50f to
20318f6
Compare
|
@swift-ci python lint |
1 similar comment
|
@swift-ci python lint |
|
@swift-ci smoke test |
1 similar comment
|
@swift-ci smoke test |
This causes build-script to use the conservative dependency information that I
committed. When one uses this option, it is assumed that one wants to also
install all built products.
Some notes:
1. I included an extra --install-all option so without --infer enabled
one can enable this sort of install everything that we want to
build behavior.
2. I added %cmake as a lit variable. I did this so I could specify in
my build-system unit tests that on Linux they should use the just
built cmake (if we built cmake due to an old cmake being on the
system). Otherwise, the build system unit tests took way too
long. These are meant to be dry-runs, so building this cmake again
is just wasteful and doesn't make sense.
3. I unified how we handle cmark, llvm, swift with the rest of the
build products by making them conditional on build_* variables, but
to preserve current behavior I made it so that they are just
enabled by default unlike things like
llbuild/swiftpm/foundation/etc. This was necessary since previously
we would just pass these flags to build-script-impl and
build-script didn't know about them. Now I taught build-script
about them so I can manipulate these skip-build-{cmark,llvm,swift}
and then just pass them down to build-script-impl if appropriate
rather than relying on build-script-impl to control if these are
built.
Once this lands, I think we are at a good enough place with
build-script until we get rid of build-script-impl in terms of high
value QoI that will imnprove productivity. Once build-script-impl is
destroyed, we can start paring back what build-script itself does.
gottesmm
force-pushed
the
pr-6a889299976ad0e3dfbc1849e1b7edaf177c53ce
branch
from
20318f6 to
a313f62
Compare
|
@swift-ci python lint |
|
I figured out the problem on Linux. We were trying to rebuild cmake. I fixed the code so we now use the just built one and also fixed the cmake check. We were comparing > instead of >= so even when I passed in the path to the just built cmake, we still tried to rebuild it since the version was the same = /. I added a >= and the problem went away. |
|
@swift-ci python lint |
5 similar comments
|
@swift-ci python lint |
|
@swift-ci python lint |
|
@swift-ci python lint |
|
@swift-ci python lint |
|
@swift-ci python lint |
|
@swift-ci smoke test |
benlangmuir
left a comment
benlangmuir
left a comment
There was a problem hiding this comment.
Having --infer imply --install-all will change the behaviour of the minimal swift build, which today does not need any installation. That may be fine, but it's not really connected to the goals of --infer. Seems worth calling out. Anyway, LGTM.
|
@benlangmuir it just really simplifies the model so I don't have to specify what needs to be installed or not. We could teach build-script about that if we want to. I am trying to do minimum viable product work here to get enough of these wins to tide us over until we can simplify more of this. |
3 participants
This causes build-script to use the conservative dependency information that I
committed. Keep in mind for this to work with toolchain dependent
build-products, we need for my install-all patch to land. Once that lands
though, we have a complete solution.