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

rustdoc: Fix source-links for files with absolute-paths #31835

Merged
merged 2 commits into from
Feb 25, 2016

Conversation

mitaa
Copy link
Contributor

@mitaa mitaa commented Feb 23, 2016

-include ../tools.mk

all:
printf "#[path=\"%s/%s\"] pub mod baz;\n" `pwd` "bar/baz.rs" > foo.rs
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this could be a unix-only rustdoc test which has a module with the path as /dev/null? That would avoid run-make and you could still check for the module page, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't think of that, but it seems like rustdoc doesn't generate files for empty public modules.

That did make another bug clear though, namely that absolute paths not contained in the root directory are still broken just like before.
(I'm gonna try to put these in a src/__root__ directory; for example /dev/null -> src/__root__/dev/null.html)

Copy link
Member

Choose a reason for hiding this comment

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

Hm ok, I just try to avoid run-make like the plague as makefiles are basically impossible to get right on both windows and unix...

@alexcrichton
Copy link
Member

@bors: r+ 78ca2e256ed86906f5d295a129f3d51e124dbbeb

@mitaa
Copy link
Contributor Author

mitaa commented Feb 23, 2016

I pushed an update so that this also works for absolute paths not contained in the root directory.

That would unfortunately be even worse to test, so I didn't include one. (works locally though)

p.to_path_buf()
}
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This seems kinda sketchy, perhaps the iterator below could use components on Path to only look at the Component::Normal parts?

@mitaa
Copy link
Contributor Author

mitaa commented Feb 23, 2016

(updated)

I've preserved the Component::ParentDir -> "up" renaming, so this does not look only at Component::Normal. (maybe this can be dropped?)

@alexcrichton
Copy link
Member

@bors: r+ 4037a7da77e529a421c4fe32e910f26abacae1af

@bors
Copy link
Contributor

bors commented Feb 24, 2016

⌛ Testing commit 4037a7d with merge 4159981...

@bors
Copy link
Contributor

bors commented Feb 24, 2016

💔 Test failed - auto-win-gnu-32-nopt-t

`clean_srcpath` tries to make the source-path relative to `src_root`,
but this didn't work since `src_root` itself wasn't absolute.
@mitaa
Copy link
Contributor Author

mitaa commented Feb 24, 2016

I think I've made your /dev/null suggestion work, removing the run-make test.

This has required commit mitaa@27ca250, because .is_file() returns false for /dev/null.
(the utility of this aside from this test seems questionable, but I hope that's okay..)
(also the same check for extern macros has been used in the same file before here, when emitting source files)

@alexcrichton
Copy link
Member

@bors: r+ cf76fcf

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 25, 2016
bors added a commit that referenced this pull request Feb 25, 2016
@bors bors merged commit cf76fcf into rust-lang:master Feb 25, 2016
@mitaa mitaa deleted the rdoc-global-src branch February 25, 2016 20:39
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.

404 for rustdoc [src] links for files with absolute paths
3 participants