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

fix running as non-root creates non-root files in the cpio #15

Closed
wants to merge 1 commit into from

Conversation

gdamjan
Copy link

@gdamjan gdamjan commented Jan 20, 2020

@@ -221,7 +221,7 @@ build_image() {
# If this pipeline changes, |pipeprogs| below needs to be updated as well.
find . -mindepth 1 -printf '%P\0' |
sort -z |
LANG=C bsdtar --null -cnf - -T - |
LANG=C bsdtar --uid 0 --gid 0 --null -cnf - -T - |
LANG=C bsdtar --uid 0 --gid 0 --null -cf - --format=newc @- |
Copy link
Author

Choose a reason for hiding this comment

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

dreisner suggested that this second line probably doesn't need the --uid/--gid, since it obviously doesn't work. should I remove those?

Copy link
Member

Choose a reason for hiding this comment

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

Might want to let @esotericnonsense look over and ensure it doesn't break reproducability.

Copy link
Member

Choose a reason for hiding this comment

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

dreisner suggested that this second line probably doesn't need the --uid/--gid, since it obviously doesn't work. should I remove those?

This second line looks weird to me, indeed.

Copy link
Member

Choose a reason for hiding this comment

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

Might want to let @esotericnonsense look over and ensure it doesn't break reproducability.

I'm going to test it also. And is reproducibility :-)

Copy link
Author

Choose a reason for hiding this comment

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

any news?

Copy link
Author

Choose a reason for hiding this comment

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

btw, doesn't the above sort -z also need a LANG=C option (just in case)?

Copy link
Member

Choose a reason for hiding this comment

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

@gdamjan
There are some more things to fix on that pipe, see #16. I'm doing a revamp of this pipeline some time this week, so you might need to rebase your patch after that.

Copy link
Author

Choose a reason for hiding this comment

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

maybe this quick fix should've been released earlier, and then do major refactors later

@gdamjan
Copy link
Author

gdamjan commented Mar 12, 2020

🔢 ?

@grazzolini
Copy link
Member

#38 implements the same feature with some more changes. I'm closing this one and I'll evaluate the other to see if it doesn't break reproducibility.

@grazzolini grazzolini closed this Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants