Skip to content
This repository was archived by the owner on May 20, 2020. It is now read-only.

Whitelist files in build.rs #112

Merged
merged 1 commit into from
Aug 8, 2017
Merged

Conversation

hjr3
Copy link
Contributor

@hjr3 hjr3 commented Aug 4, 2017

build.rs would add every file in frontend/dist to asset.in. This would
cause hard to debug errors when files were unintentionally placed in
frontend/dist.

Fixes #58

build.rs Outdated
path: String::from(entry.path().to_str().unwrap()).replace("\\", "/"),
});
let whitelist = vec![
"assets/*",
Copy link
Contributor

Choose a reason for hiding this comment

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

will this include subfolders?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This particular pattern will not. If we write it as assets/** it will include subfolders. The glob pattern syntax is here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I think that we should include subfolders as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@mgattozzi
Copy link
Contributor

@hjr3 looks like the tests are failing due to not being able to find files. I think you need to prepend "frontend/" to each of those paths in the whitelist.

@hjr3
Copy link
Contributor Author

hjr3 commented Aug 8, 2017

I believe this is ready to go. The build is still failing due to upstream issues. Let me know if there is something else I should do.

@euclio
Copy link
Contributor

euclio commented Aug 8, 2017

The build should pass with a rebase.

build.rs would add every file in frontend/dist to asset.in. This would
cause hard to debug errors when files were unintentionally placed in
frontend/dist.

Fixes steveklabnik#58
@hjr3
Copy link
Contributor Author

hjr3 commented Aug 8, 2017

@euclio indeed it does now!

Copy link
Owner

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

Thanks a ton!

@steveklabnik steveklabnik merged commit cdc586c into steveklabnik:master Aug 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants