chipyard
chipyard copied to clipboard
ucb-bar
Delete env scripts before generating
Related issue: #530
Type of change: bug fix
Impact: software change
Release Notes
Deleting the two env scripts before generating them makes manual upgrades from 1.1.0 to 1.2.0 smoother, as they were linked together in 1.1.0 but are separate in 1.2.0. When I did the manual upgrade myself, this was the main bug that I ran into that was non-obvious (as opposed to obvious things like the example generator being renamed to chipyard).
My impression was that the env.sh would be re-written / point to the env-<TOOLCHAIN>.sh when running the build-toolchains.sh script even if there was a env.sh that was already in the repo. Is this not what you saw?
What happened for me was the env.sh was symlinked to env-<TOOLCHAIN>.sh from 1.1.0, so writing to both just resulted in them having the same content (leading to infinite recursion, as per my bug report).
Thanks @baltazarortiz for your PR. We're still figuring out how the migration flow should look between versions, part of that pertains to how env.sh and friends should be regenerated.
How did you bump? Specifically, how did you update your submodules?
I'd expect that we'd want something like this to work:
git checkout <version>
$CY_ROOT/scripts/init-submodules-no-riscv-tools.sh
$CY_ROOT/scritps/build-toolchains.sh
Since env.sh is initially produced by the init submodules script and it should probably regenerate it.
No problem! I did the update a day or two after the version came out, so I don't completely remember if anything weird came up, but looking at my command history that is the sequence of commands I at least initially ran to do the bump.
It looks like I ended up running some manual git submodule commands afterwards and then had to also manually merge some custom changes, but it sounds like having run the init-submodules script should have covered the env.sh generation if things were working as intended.
It appears that both init-submodules and build-toolchains add a line env.sh (and build-toolchains also creates the toolchain specific script). However, if env.sh and the toolchain specific script are linked together as they were in 1.1.0, you'll run into the bug that I experienced.
Given this, it seems like I should switch this PR to have init-submodules rather than build-toolchains delete the scripts?
Given this, it seems like I should switch this PR to have init-submodules rather than build-toolchains delete the scripts?
That's definitely better, but if a user doesn't run the init script and instead calls git submodule X directly, they'll still run into weird bugs like the one you saw since other scripts append to env.sh. It might make more sense to give each script ownership of a single env.sh file and let the master one simply discover and source the generated ones that exist.
It seems like we need a doc page describing the upgrade process, in addition to these fixes.
How painful would it be for you to do a fresh clone when you update versions?
That would make sense! Some documentation on upgrades would definitely be useful - as someone who's not great at using submodules, getting better at them via chipyard has been a bit of an experience over the last few months :) I believe the only documentation I've seen on doing upgrades is posts to the Chipyard mailing list, which is better than nothing!
I suppose I fresh clone, reapply my generator-external changes, and then force push to my fork, though I think that means that I'd lose the commits I've made to things like RocketConfigs.scala, custom tests in the tests directory, IOBinders.scala, Top.scala, build.sbt, etc. Most of these are tiny changes, but it would still be nice to track changes (and honestly I think the main problem is that I'd forget to re-apply all the changes to my fresh clone).
If there was a way to make a generator 100% containable in a submodule rather than requiring modifications to other files and directories such as the ones I mentioned, I'd be much more open to this!
I know that would probably be yet another level of abstraction though, and I've had enough trouble figuring out Scala imports that I wouldn't even know where to start adding support for that.
Yeah that's tricky. I think what i might do if i were you initially, is create another package under generators/chipyard and instead of modifying chipyard sources, import/extend/copy in them in your package.
Then you should be able to to merge the release branch cleanly into your fork.
Ah that makes sense! I can check that out. I've been very cautiously considering switching to dev instead of tracking releases (since it looks like there have been a few bugfixes/features that might be useful), but have been a bit worried about what this would mean re: trying to keep my own code compatible.