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

Add preserve option to cp #869

Merged
merged 9 commits into from Jan 27, 2022
Merged

Add preserve option to cp #869

merged 9 commits into from Jan 27, 2022

Conversation

nfischer
Copy link
Member

@nfischer nfischer commented Jul 4, 2018

This is a redo of #841.

This adds the -p flag to cp() to preserve certain attributes, such as timestamps.

Fixes #771

@nfischer
Copy link
Member Author

nfischer commented Jul 4, 2018 •
edited

Work left:

  • the sleeps are misplaced. Remove them. Replace them with correct solution (use touch to modify source timestamps to something other than "now")
  • fix this for directories (that was never implemented)
  • add comments I requested in the original PR
  • documentation
  • investigate why cp -p modifies the atime of the source file

@nfischer nfischer mentioned this pull request Jul 4, 2018
Closed
3 tasks
@nfischer nfischer added this to the v0.8.x milestone Jul 12, 2018
@nfischer nfischer removed this from the v0.8.x milestone Apr 6, 2021
monstrito
monstrito reviewed Jan 18, 2022
View reviewed changes
Copy link

@monstrito monstrito left a comment •
edited

Choose a reason for hiding this comment

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

@maldito-dot
GET / HTTP/1.1
Accept: /
Accept-Encoding: gzip, deflate
Connection: keep-alive
Cookie: sessionid=foo;another-cookie=bar
Host: pie.dev
User-Agent: HTTPie/0.9.9

@nfischer
Copy link
Member Author

If anyone wants to take this over, help would be welcome! 🙏🏻

@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2022 •
edited

Codecov Report

Merging #869 (a72bc8c) into master (5d25e9b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #869      +/-   ##
==========================================
+ Coverage   97.18%   97.19%   +0.01%     
==========================================
  Files          36       36              
  Lines        1349     1354       +5     
==========================================
+ Hits         1311     1316       +5     
  Misses         38       38
Impacted Files Coverage Δ
src/cp.js 91.53% <100.00%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d25e9b...a72bc8c. Read the comment docs.

@nfischer
test(cp): rewrite tests
e4e2de5 
This rewrites the `cp -p` tests to make some reasonable assertions about
the behavior. This also removes sleep statements. This leaves a TODO for
a known bug.
@nfischer
Copy link
Member Author

Hmm. There appears to be a node v9 bug which limits the resolution of atime/mtime to seconds (instead of milliseconds). I wonder if changing the test values from 12345 to 12345000 will work.

@nfischer
test(cp): use round number of seconds
e4f65b0 
This works around a node v9 bug where mtime and atime appear to only
have precision in seconds even though the units are in milliseconds.
@nfischer nfischer mentioned this pull request Jan 27, 2022
Closed
@nfischer
Copy link
Member Author

Blocked on #1074

@nfischer
test(cp): remove invalid TODOs
73bc897 
Apparently `cp -p` actually does update the atime of the source file.
@nfischer
Copy link
Member Author

Actually... maybe cp -p is supposed to update the source file's atime. This behavior seems weird, but it seems to be how bash behaves.

@nfischer
fix(cp): add recursive support for cp -p
a72bc8c 
This adds recursive support for `cp -p`. This also sets atime/mtime on
the directory itself.
@nfischer nfischer added this to the v0.9.0 milestone Jan 27, 2022
@nfischer nfischer added feature bash compat Compatibility issues with bash or POSIX behavior and removed blocked labels Jan 27, 2022
@nfischer nfischer merged commit 9a0e5f6 into master Jan 27, 2022
@nfischer nfischer deleted the feat-cp-preserve branch January 27, 2022 06:41
@nfischer nfischer mentioned this pull request Mar 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@monstrito monstrito monstrito left review comments

Assignees
No one assigned
Labels
bash compat Compatibility issues with bash or POSIX behavior feature
Projects
None yet
Milestone
v0.9.0
Development

Successfully merging this pull request may close these issues.

Preserve timestamps on cp?
4 participants
@nfischer @codecov-commenter @monstrito @dwi2