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

Exclude [workspace] from cunew-generated toml #180

Merged
merged 2 commits into from
Dec 23, 2024

Conversation

mik90
Copy link
Collaborator

@mik90 mik90 commented Dec 22, 2024

I used cunew to generate a new project and hit a couple issues but this one is the one that makes most sense to fix.

I have a bit of an odd setup here. My user-side repo at $HOME/Repos/scratchpad and my Cargo workspace root is at $HOME/Repos/scratchpad/car. In my workspace, I just add all the crates in my project as members so I can build everything easily.

When I ran cunew, I ran it from the templates dir in my local copper repo, so this:

cd ~/Repos/copper-rs/templates
cargo cunew ~/Repos/scratchpad/car/copper
# Used 'git' as a copper source

When I built my code from the workspace root or the new copper project, I got this error

mike@fedora:copper $ cargo b
error: multiple workspace roots found in the same workspace:
  /home/mike/Repos/scratchpad/car/copper/car
  /home/mike/Repos/scratchpad/car

This is the reason this doesn't work: https://doc.rust-lang.org/cargo/reference/manifest.html#the-workspace-field

This field cannot be specified if the manifest already has a [workspace] table defined. That is, a crate cannot both be a root crate in a workspace (contain [workspace]) and also be a member crate of another workspace (contain package.workspace).

Omitting this element from the toml should fix it. I think that if users want this as the workspace root, they could add that in. I think this would relate to #142

Another issue I hit is that the local copper repo option only works if the generated project is in the copper repo. I hit a permission issue trying to refer to my copper repo from my new project, but I figure that might just be an intentional decision from cargo on how it manages directories/permissions anyways. Based on a skim of https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies, it doesn't seem like paths outside of repos are supported and I don't really blame them tbh.

Some conditional handling around new workspaces vs new projects in an existing workspace would make sense, but I think that having [workspace] in the toml makes the assumption that we're generating a new root workspace which isn't always true. Tell me if my understanding is wrong though.

@mik90 mik90 requested a review from gbin December 22, 2024 18:44
@mik90 mik90 changed the title Exclude [workspace] from generated toml Exclude [workspace] from cunew-generated toml Dec 22, 2024
@mik90
Copy link
Collaborator Author

mik90 commented Dec 22, 2024

So CI doesn't like this and it makes sense https://github.com/copper-project/copper-rs/actions/runs/12456810506/job/34770681064?pr=180

We could add templates/test-project to the root workspace and, even if it's not there, it works. However, I hate that. So maybe some jinja logic around the 'local' setting works because the 'local' setting implies that we're generating a new project in the existing copper workspace.

@gbin gbin merged commit b5d7700 into master Dec 23, 2024
3 checks passed
@gbin gbin deleted the mkaliman/generated-toml-fix branch December 23, 2024 11:46
@gbin
Copy link
Collaborator

gbin commented Dec 23, 2024

Thanks yes it makes sense to not create a workspace, this is just a quick starting point for the user anyway so we must accommodate the least opinionated option as possible

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.

2 participants