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

Relative paths in Cargo.toml readme #5911

Closed
kornelski opened this issue Aug 19, 2018 · 8 comments
Closed

Relative paths in Cargo.toml readme #5911

kornelski opened this issue Aug 19, 2018 · 8 comments
Labels
A-readme Area: README file issues Command-publish

Comments

@kornelski
Copy link
Contributor

I've found an odd case:

https://github.com/graphql-rust/juniper/blob/56f71e934be814722d10b0660156cf56b3753470/juniper/Cargo.toml#L12

  1. The readme path is relative pointing to a parent directory: ../README.md,
  2. there is no README.md in the .crate file,
  3. and yet, miraculously, crates.io shows a readme: https://crates.io/crates/juniper/0.9.2

It seems the readme was snatched when the crate was published, with whatever filesystem access was temporarily possible at time of publishing. Unfortunately, it means it's not possible to get the README later from the package.

Could Cargo require the README to be in the package?
@dwijnand dwijnand mentioned this issue Sep 11, 2018
Closed
@dwijnand
Copy link
Member

What should be done for this case? It's just a bit of an odd one.

@kornelski
Copy link
Contributor Author

kornelski commented Dec 19, 2018 •
edited
Loading

A simple client-side fix to Cargo would be:

  1. When publishing, validate the readme path and check that it doesn't escape crate's root (root.join(readme).strip_prefix(root).is_some() perhaps?)
  2. Check that the path is included in the package. If the crate uses include key, perhaps add it there, or warn if it's excluded.

Or rendering of README's could be moved to server-side. The server would read them from the actual package, so that authors would notice when the package is incomplete.

@dwijnand
Copy link
Member

Yeah, there's a few different ways cargo could be more consistent, the whole spectrum from hard erroring, to warning, to accommodating. I'm just not sure what we prefer. 🙂

@dwijnand
Copy link
Member

Mildly related, on publishing and README checking: #4861

@dwijnand dwijnand self-assigned this Jan 27, 2019
@dwijnand dwijnand mentioned this issue Jan 27, 2019
Closed
@dwijnand
Copy link
Member

dwijnand commented Jan 28, 2019 •
edited
Loading

In #6607 @ehuss suggests this fix:

copy the README somewhere into the package and rewrite the path in Cargo.toml

@dwijnand dwijnand removed their assignment Mar 8, 2019
@kornelski kornelski mentioned this issue Jan 25, 2020
Closed
@stanislav-tkach stanislav-tkach mentioned this issue May 10, 2020
Closed
@stanislav-tkach
Copy link
Contributor

I have just run into this issue. I think that it would be great if cargo could deal with it copying files outside of the crate directory, but in the meantime it would be nice to have a warning (personally I would prefer an error).

@stanislav-tkach stanislav-tkach mentioned this issue May 14, 2020
Open
@ehuss ehuss added the A-readme Area: README file issues label Jul 1, 2020
@erickt erickt mentioned this issue Jul 8, 2021
Open
@ehuss ehuss mentioned this issue Aug 30, 2021
Closed
@superhawk610 superhawk610 mentioned this issue Mar 17, 2022
Closed
bors bot pushed a commit to boa-dev/boa that referenced this issue Mar 17, 2022
@superhawk610
add README for crates.io publish (#1952)
520b81e 
This Pull Request closes #1948.

It changes the following:

- set `readme` in `boa_engine` so `README.md` will be published to crates.io
- remove unnecessary `exclude` field from `Cargo.toml` in all apps

I was unsure whether using a path outside of the workspace root was allowed for `readme` since it [doesn't get included in the release tarball](rust-lang/cargo#5911), but this exact path is used by [juniper](https://github.com/graphql-rust/juniper/blob/master/juniper/Cargo.toml#L13) and [seems to work there](https://crates.io/crates/juniper). I believe `cargo publish` does a bit more than just uploading the tarball, including pulling the `readme` from any arbitrary path.

The default behaviour of `cargo package`/`cargo publish` if neither `exclude` or `include` is specified is to include all files from the package root, excluding

- dotfiles
- .gitignore'd files
- subpackages (any subdirectory with a `Cargo.toml` file)
- the `/target` directory

There's no need to explicitly exclude files from the parent directory since they're already excluded by default. This can be verified by running `cargo package --list` inside any workspace app:

```plain
$ cd boa_wasm
$ cargo package --list
.gitignore
Cargo.toml
Cargo.toml.orig
src/lib.rs
```

You can read more [here](https://doc.rust-lang.org/cargo/reference/manifest.html#the-exclude-and-include-fields).
Razican pushed a commit to boa-dev/boa that referenced this issue Jun 8, 2022
@superhawk610 @Razican
add README for crates.io publish (#1952)
20f604b 
This Pull Request closes #1948.

It changes the following:

- set `readme` in `boa_engine` so `README.md` will be published to crates.io
- remove unnecessary `exclude` field from `Cargo.toml` in all apps

I was unsure whether using a path outside of the workspace root was allowed for `readme` since it [doesn't get included in the release tarball](rust-lang/cargo#5911), but this exact path is used by [juniper](https://github.com/graphql-rust/juniper/blob/master/juniper/Cargo.toml#L13) and [seems to work there](https://crates.io/crates/juniper). I believe `cargo publish` does a bit more than just uploading the tarball, including pulling the `readme` from any arbitrary path.

The default behaviour of `cargo package`/`cargo publish` if neither `exclude` or `include` is specified is to include all files from the package root, excluding

- dotfiles
- .gitignore'd files
- subpackages (any subdirectory with a `Cargo.toml` file)
- the `/target` directory

There's no need to explicitly exclude files from the parent directory since they're already excluded by default. This can be verified by running `cargo package --list` inside any workspace app:

```plain
$ cd boa_wasm
$ cargo package --list
.gitignore
Cargo.toml
Cargo.toml.orig
src/lib.rs
```

You can read more [here](https://doc.rust-lang.org/cargo/reference/manifest.html#the-exclude-and-include-fields).
@edmorley edmorley mentioned this issue Sep 29, 2022
Merged
@jmyersmsft
Copy link

jmyersmsft commented Oct 18, 2022 •
edited
Loading

Perhaps #7905 could provide inspiration and/or implementation for a fix here. I'm working on something involving .crate files separated from crates.io and there are quite a few crates that can't be fully rehydrated because there was data in the upload json that doesn't exist in the actual .crate file, the index, or (at least for older versions) the db dumps.

@kornelski kornelski mentioned this issue Dec 23, 2022
Closed
@ellie ellie mentioned this issue Apr 18, 2023
Merged
@epage
Copy link
Contributor

epage commented Oct 23, 2023

It looks like this was fixed in 3bbb781

@epage epage closed this as completed Oct 23, 2023
@guevara guevara mentioned this issue Jan 12, 2024
Open
otravidaahora2t added a commit to otravidaahora2t/boa that referenced this issue Aug 2, 2024
@otravidaahora2t @superhawk610
add README for crates.io publish (#1952)
7fed79b 
This Pull Request closes #1948.

It changes the following:

- set `readme` in `boa_engine` so `README.md` will be published to crates.io
- remove unnecessary `exclude` field from `Cargo.toml` in all apps

I was unsure whether using a path outside of the workspace root was allowed for `readme` since it [doesn't get included in the release tarball](rust-lang/cargo#5911), but this exact path is used by [juniper](https://github.com/graphql-rust/juniper/blob/master/juniper/Cargo.toml#L13) and [seems to work there](https://crates.io/crates/juniper). I believe `cargo publish` does a bit more than just uploading the tarball, including pulling the `readme` from any arbitrary path.

The default behaviour of `cargo package`/`cargo publish` if neither `exclude` or `include` is specified is to include all files from the package root, excluding

- dotfiles
- .gitignore'd files
- subpackages (any subdirectory with a `Cargo.toml` file)
- the `/target` directory

There's no need to explicitly exclude files from the parent directory since they're already excluded by default. This can be verified by running `cargo package --list` inside any workspace app:

```plain
$ cd boa_wasm
$ cargo package --list
.gitignore
Cargo.toml
Cargo.toml.orig
src/lib.rs
```

You can read more [here](https://doc.rust-lang.org/cargo/reference/manifest.html#the-exclude-and-include-fields).
@kornelski kornelski mentioned this issue Sep 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-readme Area: README file issues Command-publish
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@ehuss @epage @kornelski @dwijnand @stanislav-tkach @jmyersmsft