your-tools / tbump

Bump software releases
BSD 3-Clause "New" or "Revised" License
158 stars 22 forks source link

Using tbump with pyproject.toml and poetry #93

Closed FlorianLudwig closed 3 years ago

FlorianLudwig commented 3 years ago

Hi there,

I am playing around with tbump and i notice that it does patch my pyproject.toml

[tool.poetry]
version = "X"

without me having a [[tool.tbump.file]] entry for the pyproject.toml.

How come?

dmerejkowsky commented 3 years ago

Looks like a bug. Could you try and post a minimal example?

FlorianLudwig commented 3 years ago

To reproduce:

git clone git@github.com:FlorianLudwig/tbump-bug-93.git
cd tbump-bug-93
tbump 1.2.4

Output:

:: Bumping from 1.2.3 to 1.2.4
=> Would patch these files
- foo:1 1.2.3
+ foo:1 1.2.4
- pyproject.toml:2 version = "1.2.3"
+ pyproject.toml:2 version = "1.2.4"
- pyproject.toml:9 current = "1.2.3"
+ pyproject.toml:9 current = "1.2.4"
=> Would run these git commands
$ git add --update
$ git commit --message Bump to 1.2.4
$ git tag --annotate --message v1.2.4 v1.2.4
$ git push origin main
$ git push origin v1.2.4
:: Looking good? (y/N)
dmerejkowsky commented 3 years ago

Oh, I know what's going on. There was something in the back of my head telling me that using pyproject.toml to configure tbump was a bad idea.

You see, tbump needs to know what the current version is when patching files, so we need put the current version inside a configuration file somewhere.

That means that in addition to the files listed in [tbump.file], tbump also patches its own configuration.

The code is here:

    def compute_change_requests(self) -> List[ChangeRequest]:
        # When bumping files in a project, we need to bump:
        #  * every file listed in the config file
        #  * and the `current_version` value in tbump's config file
        assert self.config_file
        change_requests = []
        for file in self.files:
            change_request = self.compute_change_request_for_file(file)
            change_requests.append(change_request)
        rel_path = self.config_file.path.relative_to(self.working_path)
        config_file_change = ChangeRequest(
            str(rel_path),
            self.current_version,
            self.new_version,
        )
        change_requests.append(config_file_change)  # <- here
        return change_requests

So when you use tbump in patches the pyproject.toml and since tbump only knows about replacing lines, both lines in the pyproject.toml gets replaced.

I guess nobody complained so far because if you are using tbump for a python project, it makes sense to bump the version of your project in pyproject.toml too.

I guess we should use tomlkit to update tbump config file rather than re-using the code that replaces lines in source files, but that technically would be a breaking change, and you would have to patch pyproject.toml to look like this:

[tool.poetry]
version = "1.2.3"   # bumped by tbump's patch algorithm

[tool.tbump]
current = "1.2.3"  # bump by tomlkit

[[tool.tbump.file]]
src = "pyproject.toml"
# tell the patch algorithm to only replace lines containing 'version =' 
search = 'version = "{current_version}"'  

This is going to be hard to explain to existing tbump users :/

dmerejkowsky commented 3 years ago

By the way, a search on GitHub shows a list two projects that would be impacted were we to fix this bug:

https://github.com/ffreemt/deepl-fastapi

and

https://github.com/ffreemt/get-pwbrowser

FlorianLudwig commented 3 years ago

I would say tool.poetry.version could get special treatment from tbump.

The current "search and replace" everything is somewhat risky as it will at some point not just change the version in the poetry field but also a dependency.

For example:

[tool.poetry.dependencies]
python = "^3.6"
attrs = "^1.2.3"

[tool.tbump.version]
current = "1.2.3"

In this case tbump would also bump the dependency attrs which is definitely not intended.

dmerejkowsky commented 3 years ago

In this case tbump would also bump the dependency attrs which is definitely not intended.

Not if you use search = 'version = "{current_version}"' :)

I would say tool.poetry.version could get special treatment from tbump.

I don´t want to do that. If we encode something specifically for Python projects, tbump becomes less useful as a universal version bumper (which is kind of its raison d´être)

If you really want to use a tool dedicated to bump versions for Python projects, there are better alternatives :)

FlorianLudwig commented 3 years ago

Not if you use search = 'version = "{current_version}"' :)

No, that is hard-coded somewhere in tbump. Not sure where though.

dmerejkowsky commented 3 years ago

No, that is hard-coded somewhere in tbump. Not sure where though.

Yeah sorry. This is #93. When #93 is fixed, this won´t happen

FlorianLudwig commented 3 years ago

No, that is hard-coded somewhere in tbump. Not sure where though.

Yeah sorry. This is #93. When #93 is fixed, this won´t happen

FYI you are writing this in #93 ;)

dmerejkowsky commented 3 years ago

cgestes commented 3 years ago

A shadock is currently proud of you \o/