cro3
cro3 copied to clipboard
Make sure lium build and deploy can be ran without cros_workon automatically (add `--unmodified` option to them)
While reviewing the PR on add --ab_update #46, I noticed cros-workon start
is called for the packages to be deployed. Is this necessary? I feel if we do things like cros-workon
behind the scene, that might be a source of confusion.
I thought cros-workon start <package>
if for letting deploy command to use -9999
version of the package.
In practice, more developer time is wasted by forgetting cros-workon start than doing cros-workon start unintentionally, since most of the cases we do deploying a package is to push a locally-modified package.
We can avoid the confusion by printing all the commands being executed in chroot in each step I think. It is also useful for educating developers how to use the commands in the chroot and make lium as a library of working examples of commands to build & modify & test the ChromiumOS.
What do you think...?
(Maybe we should document it in our design doc and also README.md or somewhere... or maybe ARCHITECTURE.md or something?)
Hmm, okay. I didn't know that many people forget to cros-workon.
But it still leaves me a question: shouldn't that be detected when they build the package?
This is how I want the tool to behave if I forgot to cros-workon:
- edit source
- build --> lium warns me about not having cros-workon'ed the target package and let me do that interactively
- deploy
I sometimes find myself wanting to deploy the original package? I could put back the branch state, but I feel that would be more of a hassle. So for that, I want an option to skip cros-workon at deploy. But, for cases that I forget to cros-workon, I think it is better to be warned at build time.
(Well, I guess this discussion is also somewhat affected by relation to the original tool (cros) redesign and how lium want to position itself)
First, as a principle, lium should fail fast (please see our internal doc... I should move that contents to the public DESIGN.md btw...). Therefore, asking the user to do cros-workon interactively on lium build
is not an ideal option.
Also, regardless of what the cros tool is thinking, lium aims to provide a straight-forward way to "develop" chromiumos. And, in most of the cases, development is a continuous loop of change, build, deploy and test. cros-workon is just an artificial restriction made by the cros sdk, so lium should make it transparent as much as possible. That's why the current lium implementation changes the cros-workon state automatically.
How about adding "lium deploy --unmodified ..." option to ensure deploying the unmodified (crosworkon-stop-ped) packages?
Thanks on the clarification of the principle! (BTW with regard to that, there was a time I wised one of the lium commands to check the consistency of arguments before partly starting the work. However, I forgot what it was. Sorry, I will get back to it at another time)
Okay, that should probably work for me, if lium build can also build the non cros-workon version of the package.
Thank you too for asking for the clarification! Updated the issue title to make it clear what is expected. Let's work on it when someone has a free time ;)
Hmm, this was little bit more complicated than I thought.
Adding --unmodified
to build
works nicely. If I set --unmodified
, lium does cros-workon stop
and build the package. Otherwise, lium does cros-workon start
and build the package. So good, so far.
Adding --unmodified
to deploy
however turns out to be more difficult. As far as I tried, update_kernel.sh
does not obey the workon
state of the package. It seems to deploy the one that was built the last time. And it was the same for cros deploy
. it spews a warning message if I try to deploy a package without workon
. However, to my surprise, it still picks up the package with the suffix of 9999 if the last package was built during workon
. The other way around also deploys the non-intended package. So, this is confusing. In order to match the user's expectation, we would actually need to build the package that matches the given option if the last-built package was not the one we want.
(BTW, this makes me wonder if the cros-workon start
that we currently have in deploy
is really doing anything.)
I guess I can live without the --unmodified
options for now, until everybody (including the official tool) agrees on getting rid of the workon
notion from deploy
stage if ever.