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.

Sign up for GitHub

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

Jump to bottom

Support workspace only project #309

Closed
wants to merge 2 commits into from
Closed

Support workspace only project #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe8ca16
Support workspace only project
ischaojie Jun 11, 2023
e9c5722
Change to project-only
ischaojie Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions rye/src/cli/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ pub fn execute(cmd: Args) -> Result<(), Error> {
}

for project in projects {
if project.project_only() {
continue;
}
if output != CommandOutput::Quiet {
eprintln!("building {}", style(project.normalized_name()?).cyan());
}
Expand Down
8 changes: 7 additions & 1 deletion rye/src/cli/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ pub struct Args {
/// Set "Private :: Do Not Upload" classifier, used for private projects
#[arg(long)]
private: bool,
/// Only a project, not a package.
#[arg(long)]
project_only: bool,
}

/// The pyproject.toml template
Expand Down Expand Up @@ -91,6 +94,9 @@ build-backend = "pdm.backend"

[tool.rye]
managed = true
{%- if project_only %}
project-only = true
{%- endif %}

{%- if build_system == "hatchling" %}

Expand Down Expand Up @@ -237,7 +243,6 @@ pub fn execute(cmd: Args) -> Result<(), Error> {
};

let private = cmd.private;

let rv = env.render_named_str(
"pyproject.json",
TOML_TEMPLATE,
Expand All @@ -250,6 +255,7 @@ pub fn execute(cmd: Args) -> Result<(), Error> {
with_readme,
build_system,
private,
project_only => cmd.project_only,
},
)?;
fs::write(&toml, rv).context("failed to write pyproject.toml")?;
Expand Down
26 changes: 15 additions & 11 deletions rye/src/lock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,11 @@ pub fn update_workspace_lockfile(
for pyproject_result in workspace.iter_projects() {
let pyproject = pyproject_result?;
let rel_url = make_relative_url(&pyproject.root_path(), &workspace.path())?;
let applicable_extras = format_project_extras(features_by_project.as_ref(), &pyproject)?;
writeln!(local_req_file, "-e {}{}", rel_url, applicable_extras)?;
if !pyproject.project_only() {
let applicable_extras =
format_project_extras(features_by_project.as_ref(), &pyproject)?;
writeln!(local_req_file, "-e {}{}", rel_url, applicable_extras)?;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this skips installing any workspace member, but it's only the root we want to skip installing (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed, further consideration is needed here.

Copy link
Contributor

@bluss bluss Jun 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

fwiw I think your PR did something useful when I tried it, I inserted the skip root (but keep other members) here and a name in the pyproject file, and then rye sync did what was expected I think - pulling in members and dev-dependencies. Since this is all close to the pip-tools backend (which might be replaced), I think it's tricky to know how to approach this change. (I mean: whatever we implement might need to be changed later?)

local_projects.insert(pyproject.normalized_name()?, rel_url);
projects.push(pyproject);
}
Expand Down Expand Up @@ -259,16 +262,17 @@ pub fn update_single_project_lockfile(
if output != CommandOutput::Quiet {
eprintln!("Generating {} lockfile: {}", lock_mode, lockfile.display());
}

let features_by_project = collect_workspace_features(lock_options);
let applicable_extras = format_project_extras(features_by_project.as_ref(), pyproject)?;
let mut req_file = NamedTempFile::new()?;
writeln!(
req_file,
"-e {}{}",
make_relative_url(&pyproject.root_path(), &pyproject.workspace_path())?,
applicable_extras
)?;
if !pyproject.project_only() {
let features_by_project = collect_workspace_features(lock_options);
let applicable_extras = format_project_extras(features_by_project.as_ref(), pyproject)?;
writeln!(
req_file,
"-e {}{}",
make_relative_url(&pyproject.root_path(), &pyproject.workspace_path())?,
applicable_extras
)?;
}
for dep in pyproject.iter_dependencies(DependencyKind::Normal) {
writeln!(req_file, "{}", dep)?;
}
Expand Down
13 changes: 13 additions & 0 deletions rye/src/pyproject.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,11 @@ impl PyProject {
})?;
Ok(())
}

/// Is this only a workspace, not a package?
pub fn project_only(&self) -> bool {
is_project_only(&self.doc)
}
}

pub fn normalize_package_name(x: &str) -> String {
Expand Down Expand Up @@ -1107,6 +1112,14 @@ fn is_rye_managed(doc: &Document) -> bool {
.unwrap_or(false)
}

fn is_project_only(doc: &Document) -> bool {
doc.get("tool")
.and_then(|x| x.get("rye"))
.and_then(|x| x.get("project-only"))
.and_then(|x| x.as_bool())
.unwrap_or(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

would this config variable live better inside the existing group called tool.rye.workspace?

Copy link
Contributor

Choose a reason for hiding this comment

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

What about instead of adding a workspace only value we just expect members = [] where members is empty? So it's implied to be used as a workspace without members to install.

Copy link
Contributor

@bluss bluss Jun 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I guess there are different ways to go but I was thinking that a workspace-only can be used both with members and without. And it would always be a workspace, hence the workspace key should be set anyway.

(Edit, I guess to clarify, in my model the "only" does not mean there are no members. It means the project is only a workspace and not a python package. And a workspace can always have members in it.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tool.rye.workspace configuration is not normally needed for single project development. The workspace-only here might be more appropriately called no-package? or is-package = true ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Setting tool.rye.workspace makes sure that rye creates the virtualenv in that (the current) project's directory, which fits well with the "workspace-only" feature.

Do we need an additional orthogonal marker that's independent of workspace? Then you create more scenarios and additional complexity (package or "not package", workspace or not workspace is a 2x2 matrix). We can have three combinations, where a pyproject.toml becomes with rye, a:

  • python package
  • python package and a workspace
  • workspace (only)

Copy link
Contributor

Choose a reason for hiding this comment

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

To just keep the discussion diverse, I kinda think about this as everything is a workspace. Some workspaces you want to add packages/projects (members) to. Some of those projects/packages (members) are meant to be installed or their environments are managed via a workspace .venv.

So

  • python package

What keeps us from considering it as

  • python package and a workspace

and

  • workspace (only)

Is just a workspace with no added members.

Copy link
Contributor

@bluss bluss Jun 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I appreciate that. A workspace is a ceiling, or the top level of a thing. Can't include workspaces in workspaces. Then it would seem anything that is "the top" is a workspace?

}

/// Represents expanded sources.
#[derive(Debug, Clone, Serialize)]
pub struct ExpandedSources {
Expand Down