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

Wrong link to image in markdown #9939

Open
Lorak-mmk opened this issue Nov 14, 2024 · 6 comments
Open

Wrong link to image in markdown #9939

Lorak-mmk opened this issue Nov 14, 2024 · 6 comments
Labels
A-backend ⚙️ C-bug 🐞 Category: unintended, undesired behavior

Comments

@Lorak-mmk
Copy link

Current Behavior

In our README.md file we have an image:

<img src="assets/monster+rust.png" height="150" align="right">

Link to this line: https://github.com/scylladb/scylla-rust-driver/blob/f59908c54e6b6407112311a3745e32d4bd218d0c/README.md?plain=1#L1
It has not been edited for 4 years. The image is located at https://github.com/scylladb/scylla-rust-driver/blob/main/assets/monster%2Brust.png - again, in the same place for 4 years now.

Today we released new version of the crate, and the page on crates.io no longer shows the image: https://crates.io/crates/scylla/0.15.0
Image

This is because it links to https://github.com/scylladb/scylla-rust-driver/raw/HEAD/scylla/assets/monster+rust.png - notice the additional /scylla in the URL after /HEAD

All previous releases were working correctly - most recent one in September (so it is relatively recently introduced bug): https://crates.io/crates/scylla/0.14.0
Image

In this release link leads to https://github.com/scylladb/scylla-rust-driver/raw/HEAD/assets/monster+rust.png - no more /scylla after /HEAD.

Expected Behavior

I expected the image to be correctly displayed, as it always was.

Steps To Reproduce

  1. Go to: https://crates.io/crates/scylla/0.14.0
  2. Image works
  3. Go to https://crates.io/crates/scylla/0.15.0
  4. Image doesn't work despite the markdown source being the same

Environment

Not sure it is relevant here, but I'll include it anyway.

  • Browser: Firefox 130
  • OS: Fedora 39

I also verified this on Chromium

Anything else?

No response

@Lorak-mmk
Copy link
Author

Oh, I see that there was already #9886
And we do use readme = "../README.md"
But this issue basically says "this is how cargo works, sorry" - while it did work correctly 2 months ago, so it still looks like a regression.

@Turbo87
Copy link
Member

Turbo87 commented Nov 14, 2024

it did work correctly 2 months ago

huh, that's interesting. I don't remember any changes that would've caused this, but I'll take a closer look tomorrow.

it still looks like a regression.

I guess that depends on the viewpoint. My assumption is that 2 months ago we might not have taken https://docs.rs/crate/scylla/0.14.0/source/.cargo_vcs_info.json#5 into account yet (?). In your case this might look like a regression, but for people with README.md files that are actually in nested folders and don't rely on the cargo magic this would have fixed their relative paths.

@Lorak-mmk
Copy link
Author

it did work correctly 2 months ago

huh, that's interesting. I don't remember any changes that would've caused this, but I'll take a closer look tomorrow.

it still looks like a regression.

I guess that depends on the viewpoint. My assumption is that 2 months ago we might not have taken https://docs.rs/crate/scylla/0.14.0/source/.cargo_vcs_info.json#5 into account yet (?). In your case this might look like a regression, but for people with README.md files that are actually in nested folders and don't rely on the cargo magic this would have fixed their relative paths.

What is the correct / recommended way to use relative image link in our situation?

@epage
Copy link

epage commented Dec 10, 2024

Linking out to any potential cargo-related changes:

Besides moving of files, there are also symlinks. cargo package reads the target for symlinks and includes them in the .crate file. This means that if your package.readme = "README.md" is actually for a symlink to ../README.md or foo/bar/README.md, Cargo will write the content to README.md and the links could also be off

Potential options on cargo's side

  • Include in .cargo_vcs_info.json any file remapping we do, explicitly and through symlinks

@kornelski
Copy link
Contributor

For authors, I recommend using absolute image URLs in the README.

The relative URLs in README are a fiction with no basis in any web standard. Making these URLs work as expected relies on reverse-engineering custom URL schemes of 3rd party services.

The rewriting of the URLs is full of annoying edge cases due to symlinks, ../README paths, location of the crate within repo (which can be different than the location of the README), and the need to normalize .. in URLs in multiple different ways, also in a way that is host-specific. The assets may exist only at a specific commit or branch the crate has been published from, but Cargo doesn't always have this information.

Lorak-mmk added a commit to Lorak-mmk/scylla-rust-driver that referenced this issue Dec 23, 2024
crates.io doesn't handle relative links well:
rust-lang/crates.io#9939 (comment)

It is a bit sad to do this, because now you need internet to render the
README locally;

OTOH if we ever decide to change the logo, the new one will be
automatically visible in any place that uses absolute link.
@Lorak-mmk
Copy link
Author

We'll change the url to the absolute one.
It is a bit sad that relative urls don't work well, because now the link in README doesn't refer to the file in the repo in the same commit, but to the outside file that just happens to be the same because we don't change the logo.

Btw is there a way for us to test crates.io rendering without releasing a new version of the crate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

No branches or pull requests

4 participants