Skip to content

Add preserve option to cp#869

Merged
nfischer merged 9 commits intomasterfrom
feat-cp-preserve
Jan 27, 2022
Merged

Add preserve option to cp#869
nfischer merged 9 commits intomasterfrom
feat-cp-preserve

Conversation

@nfischer
Copy link
Copy Markdown
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
Copy Markdown
Member Author

nfischer commented Jul 4, 2018 •
edited
Loading

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
Copy Markdown

@monstrito monstrito left a comment •
edited
Loading

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
Copy Markdown
Member Author

nfischer commented Jan 18, 2022

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

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 26, 2022 •
edited
Loading

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
Copy Markdown
Member Author

nfischer commented Jan 26, 2022

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
Copy Markdown
Member Author

nfischer commented Jan 27, 2022

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
Copy Markdown
Member Author

nfischer commented Jan 27, 2022

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

1 more reviewer

@monstrito monstrito monstrito left review comments

Reviewers whose approvals may not affect merge requirements

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