chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.77k stars 416 forks source link

Improve how we compute and use the compilers `BUILD_VERSION` #20075

Open ronawho opened 2 years ago

ronawho commented 2 years ago

Today, the BUILD_VERSION is either a git SHA for dev or a digit for official releases. It's pretty rare that we need it for releases, but we did use it to correct a late breaking bug in 1.26.0.1 on release day. As part of that we found we weren't printing the build version except for --devel compiles. We tried to fix that in https://github.com/chapel-lang/chapel/pull/19583, but that ended up enabling printing SHAs for official releases when built in a git repo. We worked around that in https://github.com/chapel-lang/chapel/pull/20070, but the fix isn't super satisfying.

Opening this issue to brainstorm better fixes. My thought would be to have both a BUILD_VERSION and a SHA_VERSION. Use the build version for official releases if needed and use the SHA_VERSION for pre-release.

Alternatively maybe we hard code the BUILD_VERSION when marking the releases (like in https://github.com/chapel-lang/chapel/pull/20069)

ronawho commented 2 years ago

FYI @mppf @bradcray

mppf commented 2 years ago

Yes since we are editing compiler/main/version_num.h to toggle officialRelease it does not seem too off-putting to me to change static const char* BUILD_VERSION = #include "BUILD_VERSION"; into static const char* BUILD_VERSION = "0"; at the same time.

Another thing I have been wondering about (which is a bit off-topic here) is if we could avoid the changes to test/compflags/bradc/printstuff/versionhelp.sh for the release by having it grep for the value of officialRelease in version_num.h.

bradcray commented 2 years ago

My thought would be to have both a BUILD_VERSION and a SHA_VERSION. Use the build version for official releases if needed and use the SHA_VERSION for pre-release.

That seems attractive to me. I'm not particularly wed to any specific implementation approach so much as the resulting behavior. That said, I've obviously struggled and failed for years to get the behavior as I'd expect in all cases. Michael's summary in the OP of https://github.com/chapel-lang/chapel/pull/20070 sounded about right to me, though I did read it very quickly and am obviously not a reliable witness here given my many past failed attempts to improve this logic. :D

it does not seem too off-putting to me to change static const char BUILD_VERSION = #include "BUILD_VERSION"; into static const char BUILD_VERSION = "0"; at the same time.

We used to have to toggle a handful of things within the compiler to switch between official and non-official releases. It'd be attractive to me to not make that number > 1 again. That said, we could likely keep it at the same count using an #ifdef or the like, potentially replacing the "one thing to change" into a #define? Maybe we could even stop toggling that #define in the source code and just have the release-building jenkins jobs set it at build-time...?

if we could avoid the changes to test/compflags/bradc/printstuff/versionhelp.sh for the release by having it grep for the value of officialRelease in version_num.h.

I think we definitely could. There's always a question in my mind about where the right point is in the design space is between having tests be so "smart" that you end up with silent breakages and false positives that you'll never notice when something gets refactored (an example being the perennial "Could we automate the kicking of the tires of the Chapel module" question, where the whole point of that step was essentially "Do we trust our scripts and testing so completely that we'd be willing to ship a binary for Crays without having a human interact with it at all?" But none of this is to say that the current state is the ideal in any way, just where I was comfortable leaving it the last time I was involved.

mppf commented 2 years ago

Maybe we could even stop toggling that #define in the source code and just have the release-building jenkins jobs set it at build-time...?

What comes to mind for me is that we probably still want to be able to simulate a release build with a local checkout; so whatever the release-building jenkins job should do in order to enable it shouldn't be too inscrutable.

One approach we could use is to have BUILD_VERSION and SHA_VERSION, and have BUILD_VERSION store "git" or "pre-release" or some other placeholder most of the time; but then at release time (or when simulating release time) we change BUILD_VERSION to "0" or "1" etc. Then, we could have the compiler #include it (as it does today) and compute officialRelease to be true if BUILD_VERSION is not the placeholder "git" / "pre-release". That arguably keeps the compiler changes down (it's 0 or 1, depending on whether or not you count BUILD_VERSION as a compiler source code file, which technically it is).