Skip to content

Metadata transformation for the build backend#7781

Merged
konstin merged 1 commit into
mainfrom
konsti/build-backend
Oct 7, 2024
Merged

Metadata transformation for the build backend#7781
konstin merged 1 commit into
mainfrom
konsti/build-backend

Conversation

@konstin

@konstin konstin commented Sep 29, 2024

Copy link
Copy Markdown
Member

Implement the validation and transformation from pyproject.toml to METADATA and entry_points.txt. There is some incomplete plumbing code in crates/uv-build-backend/src/lib.rs to actually write METADATA and entry_points.txt into a stub wheel or dist-info directory, the logic lives in crates/uv-build-backend/src/metadata.rs.

The code is a translation of the relevant specs:

The code is a translation of these with some contextual knowledge. While there are tests, we will only have real coverage once we do the entire wheel writing process and can use them with other tools.

The feature is still hidden. Reference docs for pyproject.toml are in the next branch, to avoid publishing them.

@konstin konstin added the preview Experimental behavior label Sep 29, 2024
@konstin konstin force-pushed the konsti/build-backend branch from de23e93 to 0f110f6 Compare September 30, 2024 20:51
}

/// Allow dispatching between writing to a directory, writing to zip and writing to a `.tar.gz`.
trait AsyncDirectoryWrite: Sized {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trait will gain a method to add files

// TODO(konsti): Editables use stored.
Compression::Deflate,
)
// https://github.com/Majored/rs-async-zip/issues/150

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been considering making the build backend sync to switch to the sync zip crate

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibraheemdev ibraheemdev Oct 4, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intuition would be for the build backend to be synchronous. Unless you see some usefulness out of asynchronous control flow, async will likely just be additional complexity (and worse performance).

The other factor is, how will uv be interfacing with the build backend? If uv needs to call out to the build backend from an asynchronous context, we incur a spawn_blocking call if the build backend is synchronous. On the other hand, if the build backend is always spawned as a separate process, then async doesn't provide any benefit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the input, follow-up at: #7961

@konstin konstin force-pushed the konsti/build-backend branch from 0f110f6 to 5f53194 Compare September 30, 2024 21:34
@konstin konstin marked this pull request as ready for review September 30, 2024 21:35
@konstin konstin requested a review from BurntSushi September 30, 2024 21:35
@konstin konstin force-pushed the konsti/build-backend branch 2 times, most recently from bae43c2 to 5c7d62a Compare October 4, 2024 13:30

@BurntSushi BurntSushi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks like a reasonable start to me.

For async, I would bias toward not doing it. @ibraheemdev might have some thoughts here. My understanding is that a build backend is mostly CPU bound anyway, so using async for it might not make a ton of sense?

use std::io;
use std::path::{Path, PathBuf};
use thiserror::Error;
use uv_distribution_filename::WheelFilename;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually like to write imports in three distinct groups, separated by a line break: std imports, non-crate imports, crate imports. (I mention this because this is greenfield code and maybe we can start with this convention. :P)

/// should update the shared schema instead.
#[derive(Deserialize, Debug, Clone)]
#[serde(rename_all = "kebab-case")]
struct Project {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a dumb question (and this is for my own edification probably because I trust you're doing the right thing here), but do we have code somewhere else for parsing pyproject.toml? Why can't we use that here?

(Putting an answer to that question in the comments might be useful.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a question for @charliermarsh

}
}
Ok(Pattern::new(glob)?)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does one write a glob that matches ] and only ]?

(I ask this because I notice there is no handling of escaping here. You can escape most things via brackets, e.g., [*], but I don't think ] can be matched literally here?)

@konstin konstin Oct 7, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you can - If you consider this a concern, best add it to the discourse thread

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konstin konstin force-pushed the konsti/build-backend branch from 5c7d62a to 7797ea9 Compare October 7, 2024 08:11
konstin added a commit that referenced this pull request Oct 7, 2024
Following the discussion at #7781 (comment), it makes more sense to keep the build backend sync.
@konstin konstin mentioned this pull request Oct 7, 2024
@konstin konstin merged commit 92538ad into main Oct 7, 2024
@konstin konstin deleted the konsti/build-backend branch October 7, 2024 08:38
konstin added a commit that referenced this pull request Oct 7, 2024
Following the discussion at #7781 (comment), it makes more sense to keep the build backend sync.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Experimental behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants