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

feat(package): Extract package version from PEP 621 compliant pyproject.toml #3950

Merged
merged 5 commits into from
May 2, 2022
Merged

feat(package): Extract package version from PEP 621 compliant pyproject.toml #3950

merged 5 commits into from
May 2, 2022

Conversation

FollowTheProcess
Copy link
Contributor

Description

This PR adds a function to the package module to extract the package version from a PEP 621 compliant pyproject.toml

Motivation and Context

Closes #3949

Screenshots (if appropriate):

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Ran cargo test, cargo clippy --all-targets --all-features and cargo fmt

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

src/modules/package.rs Outdated Show resolved Hide resolved
@FollowTheProcess
Copy link
Contributor Author

I'm afraid my noob level rust is showing, I've tried a few different variations on the following but can't seem to get it to work:

fn get_pyproject_version(context: &Context, config: &PackageConfig) -> Option<String> {
    let file_contents = context.read_file_from_pwd("pyproject.toml")?;
    let pyproject_toml: toml::Value = toml::from_str(&file_contents).ok()?;

    if let Some(poetry_version) = pyproject_toml
        .get("tool")?
        .get("poetry")?
        .get("version")?
        .as_str()
    {
        format_version(poetry_version, config.version_format);
    }

    if let Some(pep621_version) = pyproject_toml.get("project")?.get("version")?.as_str() {
        format_version(pep621_version, config.version_format);
    }

    None
}

I get the following test failures:

image

So both a poetry pyproject.toml and a pep621 pyproject.toml are returning None and I can't see why. In my head the code above should check if there's a:

[tool.poetry]
version = "0.1.0"

If so, grabbing the version and formatting it.

If not, check for a:

[project]
version = "0.1.0"

If so, grabbing the version and formatting it. And if version is a table here, the .as_str() should return None according to the docs.

And finally if we get here and haven't found anything, return a None anyway.

Obviously my understanding is wrong otherwise I wouldn't be here! Can anyone help me out? 🙂

@davidkna
Copy link
Member

davidkna commented May 1, 2022

When encountering None ? will return None from the function and stop proceeding. You can either write two additional functions for both approaches to ensure the function ? does not stop execution (get_poetry_version(&pyproject_toml).or_else(|| get_pep612_version(&pyproject_toml))) or you could handle the None-values without returning, for instance with and_then.

pyproject_toml
        .get("tool")
        .and_then(|tool| tool.get("poetry"))
        .and_then(|poetry| poetry.get("version"))
        .and_then(|version| version.as_str())

@FollowTheProcess
Copy link
Contributor Author

@davidkna Worked perfectly! Thanks for the help

@FollowTheProcess
Copy link
Contributor Author

FollowTheProcess commented May 1, 2022

The way I've written this atm is it will prioritise a PEP 621 pyproject.toml so in the weird case where someone has both a [project] and a [tool.poetry], it will show the version from the [project] table and ignore the [tool.poetry] which I think is sensible behaviour as PEP 621 is the formal standard and is currently supported in a number of python build backends.

Poetry is the odd one out here and if/when it eventually adopts PEP 621, poetry pyproject.toml files will replace [tool.poetry] with [project] anyway.

davidkna
davidkna approved these changes May 1, 2022
src/modules/package.rs Outdated Show resolved Hide resolved
@andytom andytom merged commit 1b938fd into starship:master May 2, 2022
@andytom
Copy link
Member

andytom commented May 2, 2022

Thank you for your contribution @FollowTheProcess and thanks for reviewing @davidkna.

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.

Show a package version for python projects following PEP 621
3 participants