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

Dev/reproducible ordering #173

Merged
merged 2 commits into from Mar 29, 2022
Merged

Conversation

apognu
Copy link
Contributor

@apognu apognu commented Mar 28, 2022

This relates to #171.

This simple pull request enables the consistent ordering of the files embedded in the binary, to allow for consitent and reproducible builds. This uses a built-in method in walkdir. I wrote a local test to check behavior, but this relies or disorderfs which randomize entry ordering in a FUSE filesystem, so did not push it (it also only is relevant is run many times, and I don't know how to structure that with the macro used for Asset).

For reference, the test looked like this:

use rust_embed::RustEmbed;

#[derive(RustEmbed)]
#[folder = "/tmp/target"]
struct Asset;

#[test]
fn consistent_ordering() {
  let order: Vec<String> = ["a", "aa", "b", "ba", "bb", "c", "ec"].iter().map(|f| f.to_string()).collect();
  let provided: Vec<String> = Asset::iter().map(|f| f.to_string()).collect();

  assert_eq!(provided, order, "ordering should be equal");
}

I had to fix the example for poem that didn't seem to pass with the latest version the Cargo.toml resolves to, mainly:

  • The whole server and listeners are now behind a feature flag.
  • The signature for Endpoint::call() seems to have changed.

I don't know if you would prefer another PR for this, or if I can keep it mutualized into this one.

@AzureMarker
Copy link
Collaborator

It looks like the authors of poem don't understand semver. I opened poem-web/poem#246, so hopefully going forward they keep downstream users in mind. Thanks for updating the example.

Cargo.toml Outdated Show resolved Hide resolved
Copy link
Owner

@pyrossh pyrossh left a comment

Choose a reason for hiding this comment

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

LGTM

@pyrossh pyrossh merged commit 9585e1d into pyrossh:master Mar 29, 2022
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.

None yet

3 participants