Added a function that parses version information from configure.ac into build_msvc/bitcoin_config.h. This is done by default in msvc-autogen.py, so manual changing of build_msvc/bitcoin_config.h is no longer required.
In addition to that I updated the Release Process doc.
Following values are updated:
-CLIENT_VERSION_BUILD -CLIENT_VERSION_IS_RELEASE -CLIENT_VERSION_MAJOR -CLIENT_VERSION_MINOR -COPYRIGHT_YEAR -PACKAGE_STRING -PACKAGE_VERSION
fixes #23073
Sorry for the second pull request, I tried to squash the commits but something went wrong and it ended up with a lot of other commits. This is the fixed version now, hope this works.
In future, please don't open new Pull Requests for the same change. You should be able to recover your branch. Please combine the change from #23195 into this PR. You'll also need to sqaush your commits here, and write a more cohesive commit message. See the update PR description for an example.
@CallMeMisterOwl thx for the PR.
Am I correct that its purpose is to update the version fields in the build_msvc/bitcoin_config.h file with the matching values in configure.ac? If so it would be worth adding that to your PR description. It's a bit light at the moment.
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
In future, please don't open new Pull Requests for the same change. You should be able to recover your branch.
Got it
Please combine the change from #23195 into this PR. You'll also need to sqaush your commits here, and write a more cohesive commit message. See the update PR description for an example.
Done
The instructions to squash don't work when there are merge commit. You can use something like this instead:
git checkout branch_name
git fetch origin 113b863f0773999497f952daa6539a03a66a9de3
git merge 113b863f0773999497f952daa6539a03a66a9de3
git reset --soft 113b863f0773999497f952daa6539a03a66a9de3
git commit -m 'commit message'
git push origin branch_name -f
Did it work ?
No, you'll need to rebase on latest master now.
Also, you can remove the "Fixes #..." prefix from the commit message and put it into the GitHub pull request description.
Again, a rebase it not possible with merge commits in between (I think), you'll have to repeat the steps of #23198 (comment) with the commit hash of the latest master.
Works correctly for me.
The change in this PR works correctly and makes sense. But...
I'm 50-50 on the concept of the Python pre-build script for the msvc build. The original goal of the msvc build was to provide a quick and easy way for Windows devs to build and debug Bitcoin Core, adding the build to the CI was an added benefit. The dilemna is as the Python pre-build script is slowly morphing into a mini build system. It causes me a vague sesne of unease to see the script grow.
That concern noted, I've can only recall seeing two issues over nearly 4 years with devs griping about running the script so it doesn't seem to have translated into a pain point so far. This PR removes a manual step in the Bitcoin Core release process and therefore is a good idea.
tACK 8be91af6315b1d8b09a80fa35154ff453fbc9595.
Concept ACK. Seems ok to simplify the release process without complicating other steps.
@laanwj As you opened the issue, do you think this is ready for merge?
It causes me a vague sesne of unease to see the script grow.
I would agree in general. We definitely don't want to invent yet another meta-build system. But for this specifically it's quite an annoyance for every release to have to update these things manually. The reason it's not forgotten more often is because I wrote another python script (https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/make-tag.py) to check everything before making a tag. Most of that script can go after this, so I definitely see this as progress.
78 | + config_dict = dict(item.split(", ") for item in config_info) 79 | + config_dict["PACKAGE_VERSION"] = f"\"{config_dict['CLIENT_VERSION_MAJOR']}.{config_dict['CLIENT_VERSION_MINOR']}.{config_dict['CLIENT_VERSION_BUILD']}\"" 80 | + version = config_dict["PACKAGE_VERSION"].strip('"') 81 | + config_dict["PACKAGE_STRING"] = f"\"Bitcoin Core {version}\"" 82 | + 83 | + for line in fileinput.input(os.path.join(SOURCE_DIR,'../build_msvc/bitcoin_config.h', ), inplace=True):
build_msvc/bitcoin_config.h to make it clear that this script needs to be run (and to pre-empt people from filing PRs to update the stale info).build_msvc/bitcoin_config.h.in (or such) in the build system, then making a copy to build_msvc/bitcoin_config.h here. This avoids the generated state from being checked in accidentally.
Hi @laanwj,
did I understand this correctly ?
build_msvc/bitcoin_config.h.in is basically a empty template and everytime
build_msvc/msvc-autogen.py is executed it generates build_msvc/bitcoin_config.h using build_msvc/bitcoin_config.h.in and the values from configure.ac.
Yes, that sounds good to me. build_msvc/bitcoin_config.h.in would be the (read-only) template, with substitution strings that will be filled in by the script and written to build_msvc/bitcoin_config.h.
Yes, that sounds good to me.
build_msvc/bitcoin_config.h.inwould be the (read-only) template, with substitution strings > that will be filled in by the script and written tobuild_msvc/bitcoin_config.h.
Ok, I changed the code. It should work the way you mentioned now.
Code review and lightly tested ACK 410f99faed47e27fca77531a864383b6119e7b0b Thank you!
Thank you!
Pleasure contributing to this beautiful project :wink: