Skip to content

include_bytes! now registers the file included #24423

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

Merged
merged 2 commits into from
Apr 16, 2015

Conversation

tbelaire
Copy link
Contributor

This is a little bit tricky, since with include_str!, we know that we
are including utf-8 content, so it's safe to store the source as a
String in a FileMap. We don't know that for include_bytes!, but I don't
think we actually need to track the contents anyways, so I'm passing "".

new_filemap does check for the zero length content, and it should be
reasonable, howeven I'm not sure if it would be better to pass None
instead of Some(Rc::new("")) as the src component of a FileMap.

Fixes bug #24348

This is a little bit tricky, since with include_str!, we know that we
are including utf-8 content, so it's safe to store the source as a
String in a FileMap. We don't know that for include_bytes!, but I don't
think we actually need to track the contents anyways, so I'm passing "".

new_filemap does check for the zero length content, and it should be
reasonable, howeven I'm not sure if it would be better to pass None
instead of Some(Rc::new("")) as the src component of a FileMap.

Fixes bug rust-lang#24348
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

@tbelaire
Copy link
Contributor Author

Thanks @nicholasbishop, your test case was really helpful.

@alexcrichton
Copy link
Member

Thanks! Could you add a test for this as well?

@tbelaire
Copy link
Contributor Author

I have no idea how to test this, I'm sorry. Help?

@tbelaire
Copy link
Contributor Author

I mean, I did test using cargo and seeing if it rebuild the target when the input changed, but I don't know how to integrate that with rust's tests.

@nagisa
Copy link
Member

nagisa commented Apr 14, 2015

howeven I'm not sure if it would be better to pass None instead of Some(Rc::new("")) as the src component of a FileMap.

No, because then the file won’t be included into the list of dependencies.

@alexcrichton
Copy link
Member

You'll probably want to add a run-make test which generates a dep-info file with a source that has include_bytes!, and then you can check to make sure the emitted .d file indeed has the file name as expected.

This tests that both include_str! and include_bytes!  mark their input
file as a dependancy, and it's correctly outputted when you run
`rustc --emit dep-info`.
@tbelaire
Copy link
Contributor Author

Alright, I think I understand how the tests work. How this look?

@alexcrichton
Copy link
Member

@bors: r+ e4b3fac

Thanks!

bors added a commit that referenced this pull request Apr 16, 2015
This is a little bit tricky, since with include_str!, we know that we
are including utf-8 content, so it's safe to store the source as a
String in a FileMap. We don't know that for include_bytes!, but I don't
think we actually need to track the contents anyways, so I'm passing "".

new_filemap does check for the zero length content, and it should be
reasonable, howeven I'm not sure if it would be better to pass None
instead of Some(Rc::new("")) as the src component of a FileMap.

Fixes bug #24348
@bors
Copy link
Collaborator

bors commented Apr 16, 2015

⌛ Testing commit e4b3fac with merge 8f209d5...

@krh
Copy link

krh commented Mar 25, 2021

Hi, sorry to crash the discussion 6 years later, but is there a way now that this kind of dependency can be logged from custom, user-written macros? I'd like to write a macro that parses an xml spec and generates code on-the-fly, but I'd want to be able to log this kind of dependency so that when the spec is edited, the rust file that includes it is recompiled.

@jonas-schievink
Copy link
Contributor

Please ask questions on https://users.rust-lang.org/, not on the issue tracker

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.

8 participants