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.

Sign up for GitHub

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

Jump to bottom

repart: respect SOURCE_DATE_EPOCH on mkdir_p_root #32502

Merged
merged 1 commit into from May 7, 2024
Merged

repart: respect SOURCE_DATE_EPOCH on mkdir_p_root #32502

merged 1 commit into from May 7, 2024

Conversation

msanft
Copy link
Contributor

@msanft msanft commented Apr 26, 2024 •
edited by github-actions bot

This let's systemd-repart respect the SOURCE_DATE_EPOCH environment variable when creating directories in the local tree through MakeDirectories or CopyFiles.

To do this, it does two things:

  • When a directory is created in mkdir_p_root, it set's timestamps (mtime and atime) for the directory that's being created. Additionally, it will fixup the mtime for the directory things are copied into, if SOURCE_DATE_EPOCH is set. Otherwise, times are left untouched (UTIME_OMIT). The fixup of the upper directory is necessary, as otherwise, when creating a directory in it, it's mtime would change.
  • For the reason mentioned above, we also need to fixup the mtime of the upper directory if we copy a file into it.

The changes can be verified with https://github.com/msanft/systemd-repart-repro-bug.

Fixes #32117.

@github-actions github-actions bot added util-lib tests repart please-review PR is ready for (re-)review by a maintainer labels Apr 26, 2024
@github-actions GitHub Actions
Copy link

Important

An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released.

YHNdnzj
YHNdnzj requested changes Apr 26, 2024
View reviewed changes
src/basic/mkdir.c Outdated Show resolved Hide resolved
src/basic/mkdir.c Outdated Show resolved Hide resolved
src/basic/mkdir.h Outdated Show resolved Hide resolved
@YHNdnzj
Copy link
Member

YHNdnzj commented Apr 26, 2024

Please read https://systemd.io/CODING_STYLE/ first

@YHNdnzj YHNdnzj added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Apr 26, 2024
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch 2 times, most recently from 42cc9e5 to 3abe5a8 Compare April 26, 2024 13:24
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Apr 26, 2024
@msanft msanft marked this pull request as draft April 26, 2024 13:26
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch 2 times, most recently from c4bc18d to 949ca25 Compare April 26, 2024 13:35
@msanft
Copy link
Contributor Author

msanft commented Apr 26, 2024

Please read https://systemd.io/CODING_STYLE/ first

Apologies. I tried to familiarize with the guides, but let the one-line-ifs and the import slip through. Things should be in order with the style guide and contributing guidelines now.

Regarding the function signature compatability, I wasn't sure to do this, since mkdir_p_root wasn't used at a lot of places. But I now migrated to a compatibility wrapper.

@msanft msanft requested a review from YHNdnzj April 26, 2024 13:37
@msanft msanft marked this pull request as ready for review April 26, 2024 13:37
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch 5 times, most recently from b2fdf2f to de3b55c Compare April 26, 2024 14:25
@msanft msanft marked this pull request as draft April 26, 2024 14:25
DaanDeMeyer
DaanDeMeyer reviewed Apr 26, 2024
View reviewed changes
src/basic/mkdir.h Outdated Show resolved Hide resolved
DaanDeMeyer
DaanDeMeyer reviewed Apr 26, 2024
View reviewed changes
src/basic/mkdir.h Outdated Show resolved Hide resolved
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch 2 times, most recently from 4c6eb7c to c1675e2 Compare April 26, 2024 14:36
@msanft msanft requested a review from DaanDeMeyer April 26, 2024 14:45
@poettering poettering added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Apr 30, 2024
yuwata
yuwata requested changes Apr 30, 2024
View reviewed changes
src/basic/mkdir.c Outdated Show resolved Hide resolved
src/basic/mkdir.c Show resolved Hide resolved
src/basic/mkdir.c Show resolved Hide resolved
src/partition/repart.c Outdated Show resolved Hide resolved
src/partition/repart.c Outdated Show resolved Hide resolved
@yuwata yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Apr 30, 2024
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch from f0d086d to fdd0f40 Compare April 30, 2024 15:10
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Apr 30, 2024
@msanft msanft requested a review from yuwata April 30, 2024 15:10
@yuwata yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Apr 30, 2024
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch from fdd0f40 to cee41f5 Compare April 30, 2024 21:17
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Apr 30, 2024
yuwata
yuwata requested changes May 1, 2024
View reviewed changes
src/partition/repart.c Outdated Show resolved Hide resolved
@yuwata yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels May 1, 2024
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch from cee41f5 to 647a649 Compare May 6, 2024 06:08
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels May 6, 2024
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch from 647a649 to 792b5e1 Compare May 6, 2024 06:18
@msanft msanft requested a review from yuwata May 6, 2024 06:19
@msanft
repart: respect SOURCE_DATE_EPOCH on mkdir_p_root
f43b9e9 
This let's systemd-repart respect the `SOURCE_DATE_EPOCH` environment
variable when creating directories in the local tree through `CopyFiles`
or `MakeDirectories`.

To do this, we pass a timestamp `ts` to `mkdir_p_root`, which it will
use to fix up `mtime` and `atime` of the directory it creates as
well as the `mtime` of the directory it creates the other directory *in*,
as the `mtime` of the latter is modified when creating a directory in it.

For the same reason, it also needs to fixup the `mtime` of the upper
directory when copying a file into it through `CopyFiles`.

If `SOURCE_DATE_EPOCH`, times are left as is. (`UTIME_OMIT`)
@msanft msanft force-pushed the repart-mkdir-sourcedateepoch branch from 792b5e1 to f43b9e9 Compare May 6, 2024 09:52
@poettering poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels May 6, 2024
@bluca bluca merged commit 34c3d57 into systemd:main May 7, 2024
44 of 48 checks passed
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poettering poettering poettering left review comments

@bluca bluca bluca approved these changes

@yuwata yuwata yuwata approved these changes

@DaanDeMeyer DaanDeMeyer Awaiting requested review from DaanDeMeyer

@YHNdnzj YHNdnzj Awaiting requested review from YHNdnzj

Assignees
No one assigned
Labels
repart tests util-lib
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

systemd-repart does not respect SOURCE_DATE_EPOCH when creating directories in the temporary local OS tree
6 participants
@msanft @YHNdnzj @poettering @bluca @DaanDeMeyer @yuwata