Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove tomlkit from poetry-core #483

Merged
merged 5 commits into from
Apr 3, 2023

Conversation

dimbleby
Copy link
Contributor

@dimbleby dimbleby commented Sep 24, 2022

Continuation of python-poetry/poetry#6562, this removes use of tomlkit from poetry-core altogether, replacing it with tomli (and tomli-w in unit test).

tomli is smaller, faster, more stable, and being adopted by the standard library. poetry-core has no need of the extra features that tomlkit brings.

see also python-poetry/poetry#6616

@mkniewallner
Copy link
Member

I suppose that this would resolve python-poetry/poetry#6201, since we would now end up with a proper dict, and obviously make #445 unnecessary.

@dimbleby
Copy link
Contributor Author

dimbleby commented Sep 24, 2022

python-poetry/poetry#6562 alone would be enough to deal with python-poetry/poetry#6201: tomli-w will not write extra indents to the lockfile.

@neersighted
Copy link
Member

This looks good to me, but I'm not sure how best to handle it release-wise. poetry-core moving fast is all fine and good until Poetry 1.2 experiences wild changes in a patch release that breaks plugin compatibility.

Maybe we should start a new branch of poetry-core for 1.3 -- however that's a lot more effort and we'll have to change our test strategy + maintain more moving parts.

@radoering
Copy link
Member

Why should we do it the other way around here (compared to poetry)? If we can't use main for poetry 1.2 anymore, shouldn't we just create a branch from the last core version included in poetry 1.2 (currently 1.2) and only backport changes that are necessary for poetry 1.2?

Alternatively, we could hold back this PR until poetry 1.3 is in sight. Not sure if that's better...

@dimbleby
Copy link
Contributor Author

dimbleby commented Sep 25, 2022

bump the version number, update the changelog, publish. No problem.

In the relatively unlikely event that poetry-core needs a 1.2.1 release, go back and create a 1.2 maintenance branch.

(Overlapping comments, I think this is more or less what radoering said too)

@neersighted
Copy link
Member

It's more the want to set up backport infrastructure and triaging every PR as a possible regression fix that concerns me, but I suppose volume is much lower than core, so we could just cross that bridge when we come to it.

@dimbleby
Copy link
Contributor Author

IMO more or less nothing should ever be backported, just release a new version and tell folk to upgrade if they want the fix. Rare exceptions exist eg perhaps that time that the pypi API changed and left poetry completely broken, but they should be rare.

Then the fun of backporting and maintaining multiple branches pretty much goes away.

That doesn't seem to be poetry's preferred approach though. Perhaps a conversation for another time.

@sonarcloud
Copy link

sonarcloud bot commented Dec 19, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Jan 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@dimbleby
Copy link
Contributor Author

I see there's PEP-621 related activity happening, very cool.

That's surely going to have conflicts with this one, it would be good to take a decision on this one way or the other so as to avoid unnecessary work later.

@dimbleby
Copy link
Contributor Author

ugh, what's going on in the windows tests? looks like tomlkit is writing invalid toml with consecutive carriage returns?

@dimbleby
Copy link
Contributor Author

dimbleby commented Mar 21, 2023

oh no, I see, it's related to this in the unit tests

https://github.com/python-poetry/poetry/blob/71b3d6e030c879f0b4b8e3a08eac2b38a402b6bd/tests/console/commands/test_add.py#L940-L942

which indeed will transform \r\n into \r\r\n

@dimbleby
Copy link
Contributor Author

(but I don't understand where the additional carriage returns are coming from and I am somewhat hampered by the lack of a windows development environment)

@radoering
Copy link
Member

Seems to be a builtin weirdness:

Path(r"C:\temp\test.txt").write_text("test\r\n")

results in "test\r\r\n"

Maybe I can take a closer look tomorrow.

@dimbleby
Copy link
Contributor Author

Ah, I see, thanks. I withdraw my default blame-tomlkit reaction!

So the trick presumably is to specify newline="" - which is exactly what tomlkit does https://github.com/sdispater/tomlkit/blob/18a527758bfa3f8fe65e2f45f61df42f2f5af0c1/tomlkit/toml_file.py#L57-L58

pathlib's write_text() doesn't get that parameter until python 3.10, so that can't be used here.

Now fixed over in the corresponding poetry MR.

src/poetry/core/poetry.py Outdated Show resolved Hide resolved
dimbleby and others added 2 commits April 3, 2023 11:51
tomli is smaller, faster, and being adopted by the standard library.

poetry-core has no need of the extra features that tomlkit brings.
@sonarcloud
Copy link

sonarcloud bot commented Apr 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@radoering radoering merged commit 2ebc219 into python-poetry:main Apr 3, 2023
@dimbleby dimbleby deleted the strip-tomlkit branch April 3, 2023 11:21
mwalbeck pushed a commit to mwalbeck/docker-python-poetry that referenced this pull request May 23, 2023
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [poetry](https://python-poetry.org/) ([source](https://github.com/python-poetry/poetry), [changelog](https://python-poetry.org/history/)) | minor | `1.4.2` -> `1.5.0` |

---

### Release Notes

<details>
<summary>python-poetry/poetry</summary>

### [`v1.5.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#&#8203;150---2023-05-19)

[Compare Source](python-poetry/poetry@1.4.2...1.5.0)

##### Added

-   **Introduce the new source priorities `explicit` and `supplemental`** ([#&#8203;7658](python-poetry/poetry#7658),
    [#&#8203;6879](python-poetry/poetry#6879)).
-   **Introduce the option to configure the priority of the implicit PyPI source** ([#&#8203;7801](python-poetry/poetry#7801)).
-   Add handling for corrupt cache files ([#&#8203;7453](python-poetry/poetry#7453)).
-   Improve caching of URL and git dependencies ([#&#8203;7693](python-poetry/poetry#7693),
    [#&#8203;7473](python-poetry/poetry#7473)).
-   Add option to skip installing directory dependencies ([#&#8203;6845](python-poetry/poetry#6845),
    [#&#8203;7923](python-poetry/poetry#7923)).
-   Add `--executable` option to `poetry env info` ([#&#8203;7547](python-poetry/poetry#7547)).
-   Add `--top-level` option to `poetry show` ([#&#8203;7415](python-poetry/poetry#7415)).
-   Add `--lock` option to `poetry remove` ([#&#8203;7917](python-poetry/poetry#7917)).
-   Add experimental `POETRY_REQUESTS_TIMEOUT` option ([#&#8203;7081](python-poetry/poetry#7081)).
-   Improve performance of wheel inspection by avoiding unnecessary file copy operations ([#&#8203;7916](python-poetry/poetry#7916)).

##### Changed

-   **Remove the old deprecated installer and the corresponding setting `experimental.new-installer`** ([#&#8203;7356](python-poetry/poetry#7356)).
-   **Introduce `priority` key for sources and deprecate flags `default` and `secondary`** ([#&#8203;7658](python-poetry/poetry#7658)).
-   Deprecate `poetry run <entry point>` if the entry point was not previously installed via `poetry install` ([#&#8203;7606](python-poetry/poetry#7606)).
-   Only write the lock file if the installation succeeds ([#&#8203;7498](python-poetry/poetry#7498)).
-   Do not write the unused package category into the lock file ([#&#8203;7637](python-poetry/poetry#7637)).

##### Fixed

-   Fix an issue where Poetry's internal pyproject.toml continually grows larger with empty lines ([#&#8203;7705](python-poetry/poetry#7705)).
-   Fix an issue where Poetry crashes due to corrupt cache files ([#&#8203;7453](python-poetry/poetry#7453)).
-   Fix an issue where the `Retry-After` in HTTP responses was not respected and retries were handled inconsistently ([#&#8203;7072](python-poetry/poetry#7072)).
-   Fix an issue where Poetry silently ignored invalid groups ([#&#8203;7529](python-poetry/poetry#7529)).
-   Fix an issue where Poetry does not find a compatible Python version if not given explicitly ([#&#8203;7771](python-poetry/poetry#7771)).
-   Fix an issue where the `direct_url.json` of an editable install from a git dependency was invalid ([#&#8203;7473](python-poetry/poetry#7473)).
-   Fix an issue where error messages from build backends were not decoded correctly ([#&#8203;7781](python-poetry/poetry#7781)).
-   Fix an infinite loop when adding certain dependencies ([#&#8203;7405](python-poetry/poetry#7405)).
-   Fix an issue where pre-commit hooks skip pyproject.toml files in subdirectories ([#&#8203;7239](python-poetry/poetry#7239)).
-   Fix an issue where pre-commit hooks do not use the expected Python version ([#&#8203;6989](python-poetry/poetry#6989)).
-   Fix an issue where an unclear error message is printed if the project name is the same as one of its dependencies ([#&#8203;7757](python-poetry/poetry#7757)).
-   Fix an issue where `poetry install` returns a zero exit status even though the build script failed ([#&#8203;7812](python-poetry/poetry#7812)).
-   Fix an issue where an existing `.venv` was not used if `in-project` was not set ([#&#8203;7792](python-poetry/poetry#7792)).
-   Fix an issue where multiple extras passed to `poetry add` were not parsed correctly ([#&#8203;7836](python-poetry/poetry#7836)).
-   Fix an issue where `poetry shell` did not send a newline to `fish` ([#&#8203;7884](python-poetry/poetry#7884)).
-   Fix an issue where `poetry update --lock` printed operations that were not executed ([#&#8203;7915](python-poetry/poetry#7915)).
-   Fix an issue where `poetry add --lock` did perform a full update of all dependencies ([#&#8203;7920](python-poetry/poetry#7920)).
-   Fix an issue where `poetry shell` did not work with `nushell` ([#&#8203;7919](python-poetry/poetry#7919)).
-   Fix an issue where subprocess calls failed on Python 3.7 ([#&#8203;7932](python-poetry/poetry#7932)).
-   Fix an issue where keyring was called even though the password was stored in an environment variable ([#&#8203;7928](python-poetry/poetry#7928)).

##### Docs

-   Add information about what to use instead of `--dev` ([#&#8203;7647](python-poetry/poetry#7647)).
-   Promote semantic versioning less aggressively ([#&#8203;7517](python-poetry/poetry#7517)).
-   Explain Poetry's own versioning scheme in the FAQ ([#&#8203;7517](python-poetry/poetry#7517)).
-   Update documentation for configuration with environment variables ([#&#8203;6711](python-poetry/poetry#6711)).
-   Add details how to disable the virtualenv prompt ([#&#8203;7874](python-poetry/poetry#7874)).
-   Improve documentation on whether to commit `poetry.lock` ([#&#8203;7506](python-poetry/poetry#7506)).
-   Improve documentation of `virtualenv.create` ([#&#8203;7608](python-poetry/poetry#7608)).

##### poetry-core ([`1.6.0`](https://github.com/python-poetry/poetry-core/releases/tag/1.6.0))

-   Improve error message for invalid markers ([#&#8203;569](python-poetry/poetry-core#569)).
-   Increase robustness when deleting temporary directories on Windows ([#&#8203;460](python-poetry/poetry-core#460)).
-   Replace `tomlkit` with `tomli`, which changes the interface of some *internal* classes ([#&#8203;483](python-poetry/poetry-core#483)).
-   Deprecate `Package.category` ([#&#8203;561](python-poetry/poetry-core#561)).
-   Fix a performance regression in marker handling ([#&#8203;568](python-poetry/poetry-core#568)).
-   Fix an issue where wildcard version constraints were not handled correctly ([#&#8203;402](python-poetry/poetry-core#402)).
-   Fix an issue where `poetry build` created duplicate Python classifiers if they were specified manually ([#&#8203;578](python-poetry/poetry-core#578)).
-   Fix an issue where local versions where not handled correctly ([#&#8203;579](python-poetry/poetry-core#579)).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS44Mi4wIiwidXBkYXRlZEluVmVyIjoiMzUuODIuMCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9-->

Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/717
Co-authored-by: renovate-bot <[email protected]>
Co-committed-by: renovate-bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants