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

Detect incorrectly named cargo.toml #9541

Closed
pnkfelix opened this issue Jun 3, 2021 · 12 comments · Fixed by #9607
Closed

Detect incorrectly named cargo.toml #9541

pnkfelix opened this issue Jun 3, 2021 · 12 comments · Fixed by #9607
Assignees
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` E-easy Experience: Easy

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Jun 3, 2021

A new user following tutorials won't necessarily use cargo new; they may create files by hand.

In practice, I have found that such users may accidentally create cargo.toml instead of Cargo.toml.

If Cargo.toml is missing, cargo should detect the presence of a file like cargo.toml and suggest that the user rename it to Cargo.toml.

@pnkfelix pnkfelix added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jun 3, 2021
@ehuss ehuss added A-diagnostics Area: Error and warning messages generated by Cargo itself. E-easy Experience: Easy labels Jun 3, 2021
@hedonhermdev
Copy link

Hey can I take this up?

@ehuss
Copy link
Contributor

ehuss commented Jun 4, 2021

Of course, let me know if you have any questions. There is a contributor guide here if you need more information on working on cargo.

Also, just for the future, you are welcome to automatically claim an issue yourself (as noted here, you can use @rustbot claim if you want to make it formal).

@hedonhermdev
Copy link

Should I also add a test for this?

@ehuss
Copy link
Contributor

ehuss commented Jun 4, 2021

Yea, that would be good! We try to add tests for every addition and fix when possible.

@hedonhermdev
Copy link

Hey, so the filesystem on MacOS is case-insensitive and treats cargo.toml and Cargo.toml as the same. So I'm not sure how to test it.

@ehuss
Copy link
Contributor

ehuss commented Jun 6, 2021

The test could create a file (Test) and then try to open it with lowercase (test) and if that succeeds, you'll know you're on a case-insensitive filesystem. I'm not sure how you're planning to implement it for that case, if you will silently allow it, issue a warning, or an error. I'm thinking in the spirit of the issue, it might be good to detect it, but I'm not sure how difficult that would be.

@Rustin170506
Copy link
Member

Rustin170506 commented Jun 15, 2021

@hedonhermdev Are you still fixing this issue? I want to try to improve it! If you are not trying to fix it, could you please let me know?

@hedonhermdev
Copy link

Hey yes you can continue with it

@Rustin170506
Copy link
Member

@ehuss Could you offer some help please? I seem to be having trouble finding a place to add that suggestion. Is there a place where these commands uniformly parse Cargo.toml that can be modified?

@ehuss
Copy link
Contributor

ehuss commented Jun 16, 2021

I think it might go here where it is loaded, and just check for a NotFound error.

@Rustin170506
Copy link
Member

@rustbot claim

@rustbot rustbot assigned Rustin170506 and unassigned hedonhermdev Jun 17, 2021
@Rustin170506
Copy link
Member

I think it might go here where it is loaded, and just check for a NotFound error.

There still seems to be a lot of places where that path is being used. I try to try to warn them uniformly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` E-easy Experience: Easy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants