Skip to content

Set file permissions when the archive is created from STDIN#5437

Closed
schors wants to merge 0 commit intoborgbackup:masterfrom
schors:master
Closed

Set file permissions when the archive is created from STDIN#5437
schors wants to merge 0 commit intoborgbackup:masterfrom
schors:master

Conversation

@schors
Copy link
Contributor

@schors schors commented Oct 25, 2020

--stdin-mode, --stdin-user and --stdin-group added
File mode forces to regular file with other values from the option --stdin-mode. Default mode is 0o100660.
Error if given user or group do not exists. Default user and group values are taken from system uid/gid 0

@schors
Copy link
Contributor Author

schors commented Oct 26, 2020

I am going to fix issues from #8844.8 check job in a one-two days. But I don't understand what I can do with FAILED test from #8844.5 check job. Everything looks well. I don't know how to reproduce check environment in an acceptable time. I need help

@ThomasWaldmann
Copy link
Member

Thanks for the PR. Yes, please fix the pep8 issues.

You can ignore #5438, that has nothing to do with your changes and needs to be addressed separately.

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Oct 26, 2020

And please clean up the commits, there should be no "merge" in a PR and also we need good/clear commit comments that are suitable to create a changelog entry from.

You can (carefully!) do that with git rebase -i master (and update your local master first).

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

looks pretty good! a few minor nitpicks which i have found...

UMASK_DEFAULT = 0o077

# default file mode to store stdin data, defaults to read/write for owner and group
# forsing to 0o100XXX later
Copy link
Member

Choose a reason for hiding this comment

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

typo: "forcing" (also fix that globally, please)

Comment on lines +3232 to +3237
subparser.add_argument('--stdin-user', metavar='USER', dest='stdin_user', default=uid2user(0),
help='set user USER in archive for stdin data (default: %(default)r)')
subparser.add_argument('--stdin-group', metavar='GROUP', dest='stdin_group', default=gid2group(0),
help='set group GROUP in archive for stdin data (default: %(default)r)')
subparser.add_argument('--stdin-mode', metavar='M', dest='stdin_mode', type=lambda s: int(s, 8), default=STDIN_MODE_DEFAULT,
help='set mode to M in archive for stdin data (default: %(default)04o)')
Copy link
Member

Choose a reason for hiding this comment

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

cool, that's pretty much how i would have solved it! :-)

mode=0o100660, # regular file, ug=rw
uid=uid, user=uid2user(uid),
gid=gid, group=gid2group(gid),
mode=mode&0o107777|0o100000, # forsing regular file mode
Copy link
Member

Choose a reason for hiding this comment

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

if you set the regular file bit anyway (via "or"), guess you do not need to have it in the "and" mask.

@ThomasWaldmann
Copy link
Member

@schors did you see my feedback?

@schors
Copy link
Contributor Author

schors commented Oct 31, 2020

@ThomasWaldmann yes, I did. I am very busy at the moment. I have any free time only on weekends. I'm going to do something related this issue tomorrow.

@codecov-io
Copy link

codecov-io commented Nov 1, 2020

Codecov Report

Merging #5437 into master will decrease coverage by 0.09%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5437      +/-   ##
==========================================
- Coverage   83.69%   83.60%   -0.10%     
==========================================
  Files          37       37              
  Lines        9957     9973      +16     
  Branches     1656     1659       +3     
==========================================
+ Hits         8334     8338       +4     
- Misses       1140     1149       +9     
- Partials      483      486       +3     
Impacted Files Coverage Δ
src/borg/archive.py 82.42% <42.85%> (-0.63%) ⬇️
src/borg/archiver.py 81.31% <91.66%> (-0.06%) ⬇️
src/borg/constants.py 100.00% <100.00%> (ø)
src/borg/helpers/parseformat.py 89.00% <0.00%> (-0.17%) ⬇️
src/borg/repository.py 84.20% <0.00%> (+0.18%) ⬆️

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 5bcb52d...32b7c1d. Read the comment docs.

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.

3 participants