-
Notifications
You must be signed in to change notification settings - Fork 389
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
File permission transfer #155
Changes from 3 commits
d08d3e9
8e671b7
b85b559
93e61d3
90dafff
b6def07
93ed192
eed6c64
37ddca5
5b1af9b
6a9dae2
e3c2c6a
7bb17e3
0125fe6
b57a4fc
caf98ff
ae240d9
3f889bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,15 @@ if [ -z "$WDT_TEST_SYMLINKS" ] ; then | |
fi | ||
echo "WDT_TEST_SYMLINKS=$WDT_TEST_SYMLINKS" | ||
|
||
# Set WDT_TEST_PERMISSION: | ||
# to 1 : check if permissions match | ||
# to 0 : when "find -printf %m" does not work, i.e. on Mac | ||
if [ -z "$WDT_TEST_PERMISSION" ] ; then | ||
WDT_TEST_PERMISSION=1 | ||
umask 0 | ||
fi | ||
echo "WDT_TEST_PERMISSION=$WDT_TEST_PERMISSION" | ||
|
||
# Verbose / to debug failure: | ||
#WDTBIN="_bin/wdt/wdt -minloglevel 0 -v 99" | ||
# Normal: | ||
|
@@ -96,6 +105,10 @@ do | |
-directory="$DIR/src" -filename="$base.$i" -gen_size_mb="$size" | ||
done | ||
done | ||
|
||
if [ $WDT_TEST_PERMISSION -eq 1 ]; then | ||
chmod 01777 $DIR/src/inp.0009765625.1 | ||
fi | ||
echo "done with setup" | ||
|
||
if [ $WDT_TEST_SYMLINKS -eq 1 ]; then | ||
|
@@ -137,7 +150,7 @@ if [ $DO_VERIFY -eq 1 ] ; then | |
echo "Should be no diff" | ||
(cd $DIR; diff -u src.md5s dst.md5s) | ||
STATUS=$? | ||
if [ $STATUS -eq 0 ] ; then | ||
if [ $STATUS -eq 0 ] && [ $WDT_TEST_PERMISSION -eq 1 ]; then | ||
(cd $DIR/src ; ( find . -type f -printf %f%m\\n | sort ) > ../src.perm ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently within the /tmp/wdtTest_[usrname], there are several files with 664, which include ".a" and ".b" if I remember correctly. Regarding Mac... I am not sure whether we need to consider that. @uddipta Could you clarify this? Similarly I think stat() might not work on Windows. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
So, we need to add a file with different permission for testing. As Jason mentioned before, current find cmd does not work on mac. So, we can disable the test for now on Mac. You can do that using an environment variable. We can have an environment variable like SKIP_PERMISSION_TEST and set it in https://github.com/facebook/wdt/blob/master/build/travis_osx.sh. If you have access to a mac, you can also try to make the test work for mac. Right now, find is failing for mac, but we are not checking the return code. Both, src.perm and dst.perm are empty, so the test is passing. Also, run clangformat.sh once. Every time you update this PR, you should see a travis build/test triggered. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
(Part of "ls -l" output on the src dir is here.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is better to chmod a file to have permission like 777. Just do that for file1. Regarding adding test for mac, we will add it later. |
||
(cd $DIR/dst ; ( find . -type f -printf %f%m\\n | sort ) > ../dst.perm ) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to set this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are asking about why umask? Some machines (including mine) have a mask by default, which prevents giving write permission to Others. If we do not umask it, the file created on the receiver side will not have the write perm even if it is a 0777 on the writer side.