mbarkhau / bumpver

BumpVer: Automatic Versioning
https://github.com/mbarkhau/bumpver
MIT License
199 stars 36 forks source link

CLI flag to bump version based from working dir instead latest VCS tag #188

Closed demmerichs closed 1 year ago

demmerichs commented 2 years ago

The title says it all.

I read the reasoning in the README under 'Bump it up' for making the decision to take as the current version the most recent one from the VCS tags, globally across all branches.

But there are use-cases, especially with release branches, where the current version should be taken from the current working directory. This is already shown in bumpver output, however I was expecting a CLI flag to switch this behavior if needed, but could not find it. More frustrating even is, that setting the version manually won't work either, as bumpver still compares this against the most current version it found.

In case you are interested, the use case I am facing currently: Stable/main branch on version 1.0.1 Release branch with additional features on version 1.1.0rc2 In the mean time I want to push some minor fixes to the stable branch, e.g. incrementing it to 1.0.2. To me it seems this kind of branch structure might be quite common so I was wondering, why bumpver is not supporting this kind of behavior.

demmerichs commented 2 years ago

The work around I now used successfully, is to delete the tag of the newer 1.1.0rc2 only locally, then do a bumpver update with the -n flag (no fetching). Afterwards a bumpver show retrieves the locally deleted tag again from the origin.

git tag -d 1.1.0rc2
bumpver update -n --patch
bumpver show
mbarkhau commented 2 years ago

Sounds like a reasonably common use-case. Out of curiosity, for your use-case, I would assume ideally, bumpver would only consider tags that are for an ancestor of the current HEAD?

Suggestion for the CLI parameter --ssot=(global | branch | config)

Not sure about the name, but that's the idea. You have any interest in working on a PR for this?

demmerichs commented 2 years ago

Hi, thanks for the response. For me, your branch suggestion sounds like the thing I want for my workflow yes! Maybe I would even then consider adding a fourth option ignore, that could be used together with --set-version in case somebody wants to get completely wild. Not sure about the ssot name either, as it is an uncommon abbreviation in general, and does not even specifically mention which kind of 'truth' (current version) we are talking about. As it might be a less used feature based on me being the first to complain, I think a longer name, maybe along the lines of --ref-version or perhaps --relative-to is more appropriate. Not really sure about those either :D Naming is hard.

I might be interested in working on a PR for this, thanks for the suggestion, but it is not going to happen soon, the earliest would probably be mid-November or even later, over the Christmas holidays. So if anyone else wants to have a go at this at an earlier time, I don't want to stop them.

demmerichs commented 2 years ago

I just thought of maybe even a more wholesome solution to this problem. The 'SSOT' idea/argument for choosing the global strategy was to avoid version collisions, right? When choosing the branch strategy, an automatic check of version collision of all global tags would still be nice, but probably isn't implemented yet, as the global strategy ensures that version collision on different branches can not happen. I think the check against all version tags is computationally feasible, as the user accepts the wait of a second for fetching the tags as well. Wouldn't this then just be a more sane default anyway, always using the branch strategy, e.g. only comparing against ancestors, but additionally doing a version collision check of some sort? Or are there other reasons that were considered when deciding to go for the global SSOT philosophy?

malnvenshorn commented 1 year ago

I'm interested in working on a PR for this, as I'm running into the same issue right now.

$ git checkout dev

$ git tag --list --merged
0.1.0
0.2.0b0

$ git checkout master

$ git tag --list --merged
0.1.0
0.1.1

$ bumpver update --no-fetch --patch --dry
INFO    - Working dir version        : 0.0.0
INFO    - Latest version from VCS tag: 0.2.0b0
INFO    - Old Version: 0.2.0b0
INFO    - New Version: 0.2.1b0

I would implement it the way as @demmerichs suggested:

  1. Taking the current_version from the checked out branch using git tag --list --merged
  2. Checking the new_version against all tags to avoid collisions
  3. Throwing an error if a collision happens

I also would make this behavior the default and don't even bother adding a --ref-version=(global | branch | config) parameter unless @mbarkhau wants to. I can't think of why someone would use the global in a different branch and according to the documentation the git tags are the single source of truth, therefore config seems strange (see #159). But that's not for me to decide :laughing:

I would appreciate some feedback on this.

demmerichs commented 1 year ago

Hi, I totally forgot about this potential PR. Thanks for picking it up again :)

Your proposal sounds good to me and I would be fine with it for now. However, I think adding the flag would be the "better" more long-term solution, for a couple of reasons:

  1. Backward compatibility: We might not know if anyone actually depends on the "global" strategy and built automatism around this "feature". As a project dev, I would be scared to change this in a minor version. So having the flag, and global as default up until the next major version change of bumpver would be the "nicer" thing to do.
  2. Workaround in error case: I could imagine the error case of a collision actually happening without any bigger mistakes in project management, e.g. when main is updating its minor and from 1.1 to 1.2 and the original 1.2 release is postponed to 1.3 in a later commit. It would be nice to simply throw in a flag for this one-timer in order to move along, instead of finding a work-around (again). For this case I would also like to reiterate my idea of adding a fourth option ignore and another flag allowing to set the version directly on the CLI.
  3. Flexibility: Even though tools like bumpver are supposed to unify the way versioning is done across projects, there might be good reasons for using a global or config reference mode. So if feasible, I tend to vote for the additional configuration with a sane default, instead of a single hard-coded way of running things.

I really hope, the owner responds to this and gives you a green light for this, whichever shape this feature will take form :)

malnvenshorn commented 1 year ago

Hi, 1. and 3. sounds plausible to me. As for the second point: I tend to disagree and wouldn't add some kind of --force flag to allow collisions. If you have a collision not be accident, there might be something wrong with the versioning. Version 1.2 is a stable release. If it's a work in progress it should be named 1.2-dev, 1.2a0 or something like that. If you postpone those change to a later release just update the version to 1.3-dev und just release 1.2 on the main branch.

mbarkhau commented 1 year ago

I've not had time to fully consider the suggestions by @malnvenshorn, so I'll just give some feedback for consideration.

Skimming over it, reading "make this behavior the default", I'd just caution to think about maintaining backward compatibility. I don't want any CI pipelines to break because of a change we make here.

git tags are the single source of truth, therefore config seems strange

So long as there are tags, they will be used. You can use bumpver without version control and also when there are no tags yet, in which case the config is used. So in principle the config is for bootstrapping and is ignored once tags are available.

git tag --list --merged

My concern here would be the possibility of generating the same version number/tagging two different revisions, on different branches. Another minor concern would be maintaining support for mercurial, but I wouldn't reject the PR, so long as functionality is strictly expanded (i.e. worst case nothing improves for mercurial users, but nothing gets worse either).

malnvenshorn commented 1 year ago

You can use bumpver without version control

I wasn't aware of that. Thanks for pointing this out.

My concern here would be the possibility of generating the same version number/tagging two different revisions, on different branches.

A check of new_version against git tag --list is mandatory to avoid this, even if the new_version is derived only from tags of the current branch.

Another minor concern would be maintaining support for mercurial

I have no experience with mercurial, but according to this stackoverflow question the mercurial equivalent would be hg log --rev="branch(THISBRANCH) and tag()" --template="{tags}\n" but haven't tested this yet. Otherwise nothing would change for mercurial users and --ref-version=branch will be ignored.

demmerichs commented 1 year ago

I would probably focus on git and drop mercurial support for this. I think most users use git, and glancing over a short comparison of git and mercurial, it seems to me the latter is not intended to be used in a branching model, therefor this feature might be less relevant for such users.

I'd be happy to also give feedback/do a code review for a potential PR, or assist in minor ways during development.

malnvenshorn commented 1 year ago

I looked a bit into the source code and some further questions came up.

Quoting the documentation

bumpver treats Git/Mercurial tags as the canonical / SSOT for the most recent version

but that's not the case actually. bumpver takes the newer version either from config or VCS tags.

https://github.com/mbarkhau/bumpver/blob/277d893de3674b7231f104e6567e73bbadf3a8d9/src/bumpver/cli.py#L678-L680

Naming the parameter --ref-version or --ssot would be misleading. I think something like --vcs-tag-scope or --git-tag-scope=(global | branch) (if we decide to drop mercurial support for this) is more appropriate.

As you can see I'd also drop the config option for this parameter. If we allow the config as single source of truth this might mess up the version in the VCS. ~Only allowing --git-tag-scope=(global | branch) would totally preserve the current behavior as the only change is how the current version from the VCS is taken~ (see edit). And if you are using bumpver without a VCS nothing will change at all. If you need to manually force a version there is still --set-version with --ignore-vcs-tag.

Edit: Ok well that was bullshit :laughing: The behavior has to change as VCS tags must be the ssot to make this work.

malnvenshorn commented 1 year ago

Just to point that out again: this change will break current behavior. Let me explain this in two examples.

You have two branches master with current version 1.1.0 and dev with 1.2.0b1. Now you want to include some of the changes from the dev branch in a 1.1.1 release. So you pick pocket commits from the dev branch, but one of them includes changes to the pyproject.toml which is the config file for bumpver. To update the version and tag on the master using bumpver update --patch the VCS tag must be the single source of truth and the version in the config file (1.2.0b1) must be ignored.

But this breaks existing behavior, because the results of the following commands would be different. Some example with the current behavior:

$ bumpver update --patch --no-tag-commit
INFO    - Working dir version        : 1.1.0
INFO    - Latest version from VCS tag: 1.1.0
INFO    - Old Version: 1.1.0
INFO    - New Version: 1.1.1
INFO    - git commit --message 'bump version 1.1.0 -> 1.1.1'

$ bumpver update --patch
INFO    - Working dir version        : 1.1.1
INFO    - Latest version from VCS tag: 1.1.0
INFO    - Old Version: 1.1.1
INFO    - New Version: 1.1.2
INFO    - git commit --message 'bump version 1.1.1 -> 1.1.2'
INFO    - git tag --annotate 1.1.2 --message 1.1.2

But with tags as single source of truth the result of the second command would be:

$ bumpver update --patch
INFO    - Working dir version        : 1.1.1
INFO    - Latest version from VCS tag: 1.1.0
INFO    - Old Version: 1.1.0
INFO    - New Version: 1.1.1
INFO    - git commit --message 'bump version 1.1.0 -> 1.1.1'
INFO    - git tag --annotate 1.1.1 --message 1.1.1

I'm open for thoughts and feedback!

demmerichs commented 1 year ago

So to summarize: The current behavior is something like old version = max(global tags, config). Our originally intended behavior would be old version = max(branch tags).

I think two questions/decisions arise from this:

  1. Is the current behavior a bug, as it contradicts the documentation, or is it a feature that is not documented? I am unsure about this. Bumpver is supposed to work without VCS as well, therefor it needs to fall back to config in those environments. But I could also imagine users wanting to update the version without commit and later further updating with commit and tag, which would also be in favor of the current implementation. Either way, this observation should be addressed either through a doc update or a fix towards old version = config if (global tags).empty() else max(global tags).
  2. How should it work for the branch setting? Here I would keep it consistent with whatever answer to the first we come up with.

I am leaning towards keeping the current behavior and introducing it also to the branch setting, mostly because of backwards compatibility reasons. About your example of pick pocketing/cherry picking commits from a dev branch to master, I think the workflow would still be working, because: The version commits are usually separate commits. If you cherry pick a row of commits, I would manually remove the version upgrade commits from the ones cherrypicked. This should guarantee, that the config file version number stays on 1.1.0 in your example. In my opinion the commit history would be a bit confusing if the intended workflow would be to include those commits in the cherrypick, and then have a history going from 1.1.0 to 1.2.0b1 back to 1.1.1.

malnvenshorn commented 1 year ago

I agree with you that we should keep the existing behavior of old version = max(global tags, config) because the user is not forced to commit and tag the version bump even if a VCS exists in the project and bumpver needs to keep track of those.

The version commits are usually separate commits.

Not necessarily as you can run bumbver with the --no-commit flag. This means the updated version could be in the same commit where you updated the dependencies for example.

This still leaves the question how we should handle such cases.

demmerichs commented 1 year ago

Mh, so if I would bump the version with no commit, I would not put the version change in a later commit either, kind of squashed with something else, because that seems an antipattern to bumpver. And to all the users, doing this kind of antipattern, they probably have for themselves already some kind of work-arounds for those cases you described.

Just to list a few workarounds: During cherrypicking also editing the commit to remove the version change, or do an interactive rebase after the cherry picking. I believe you can also just change the version line to something old in the config before calling bumpver, thus bumpver falling back to the newest tag. But of course those are workarounds and it would probably be nicer to change the behavior again with a CLI flag, like --ignore-cfg similar to --ignore-vcs-tag. BTW, I liked your flag proposal: --vcs-tag-scope

malnvenshorn commented 1 year ago

But of course those are workarounds and it would probably be nicer to change the behavior again with a CLI flag, like --ignore-cfg similar to --ignore-vcs-tag

I think that's a good solution, if @mbarkhau is ok with this.

mbarkhau commented 1 year ago

Thanks @demmerichs and @malnvenshorn for thinking things through here.


First some answers/comments.

I would probably focus on git and drop mercurial support for this.

Once the PR is working for git, I'll have a look if mercurial support isn't too difficult to add.

Is the current behavior a bug, as it contradicts the documentation?

The current behaviour is not a bug. If there is a discrepancy, the documentation is wrong.


Now I'll try to summarise what I believe we can agree on.

The user should have control over what is considered the current_version. The cli argument for this seems best to me: --tag-scope=(default|global|branch).

We could implement this with some fancy comma separation and give the user full control, but I'm inclined to not introduce a foot-gun.

--tag-scope= current_version =
default max(config.current_version, max(global_vcs_tags))
global max(global_vcs_tags)
branch max(branch_vcs_tags)

When using --tag-scope=branch, an error should be generated if the updated version would conflict with a tag on another branch.

malnvenshorn commented 1 year ago

Looks good to me. I guess bumpver should still default to config.current_version for --tag-scope=branch and --tag-scope=global in the case no tags exists?

mbarkhau commented 1 year ago

Looks good to me. I guess bumpver should still default to config.current_version for --tag-scope=branch and --tag-scope=global in the case no tags exists?

Yes, that seems appropriate.

mbarkhau commented 1 year ago

Fixed with bumpver==2023.1125.