Added support for fifos and sockets (#42) #204
Conversation
mgree
left a comment
mgree
left a comment
There was a problem hiding this comment.
Nearly there! A few nits, but mostly just the test fix and the find_upperdir_changes/process_changes fix.
Vagrantfile
Outdated
| sudo modprobe overlay | ||
| cd try | ||
| autoconf && ./configure --disable-utils |
There was a problem hiding this comment.
Everywhere else, this is ... && make. Make it the same?
| AUTO_CFLAGS="" | ||
| fi | ||
| AUTO_CPPFLAGS="" | ||
| AUTO_CPPFLAGS="" |
There was a problem hiding this comment.
This should go now, since we've set it unconditionally above.
configure.ac
Outdated
| if test -z "$CFLAGS" | ||
| then | ||
| AUTO_CFLAGS="-g -Wall -O2" | ||
| AUTO_CPPFLAGS="-g -Wall -O2" |
There was a problem hiding this comment.
These are not sensible C preprocessor flags. We want to set AUTO_CPPFLAGS to "" unconditionally.
| AUTO_CPPFLAGS="" | ||
| AUTO_CPPFLAGS="" | ||
|
|
||
| CFLAGS=${CFLAGS-"$AUTO_CFLAGS"} |
There was a problem hiding this comment.
These lines need to be moved up, so that we're actually setting CFLAGS and CPPFLAGS!
test/all-commit-cases.sh
Outdated
|
|
||
| cleanup() { | ||
| cd / | ||
| cd / |
There was a problem hiding this comment.
This file has been reformatted... can you fix it so it's only the real diff? 🥲
|
|
||
| : $((COUNT += 1)) | ||
|
|
||
| ! [ -e newsock ] || fail |
There was a problem hiding this comment.
This does not test the declared case.
|
|
||
| : $((COUNT += 1)) | ||
|
|
||
| ! [ -e newsock ] || fail |
There was a problem hiding this comment.
This does not test the declared case.
try
Outdated
|
|
||
| find "$sandbox_dir/upperdir/" -type f -o \( -type c -size 0 \) -o -type d -o -type l | ignore_changes "$ignore_file" | ||
| } | ||
| find "$sandbox_dir/upperdir/" \( -type f -o -type c -size 0 -o -type d -o -type l -o -type s -o -type p \) | ignore_changes "$ignore_file" |
There was a problem hiding this comment.
Why are we filtering by type at all, at this point? This list omits only -b and non-empty -c... both of which are mistakes, I think. Since we can handle any type of file, we don't need to filter in the find at all... right?
try
Outdated
| continue | ||
| fi | ||
|
|
||
| elif [ -p "$changed_file" ] |
There was a problem hiding this comment.
This is mis-indented, should be two back.
try
Outdated
|
|
||
| elif [ -p "$changed_file" ] | ||
| then | ||
| if [ -e "$local_file" ] |
There was a problem hiding this comment.
All the logic for this case seems basically identical to the logic for regular files below... merge the branches?
Vagrantfile
Outdated
| sudo modprobe overlay | ||
| cd try | ||
| autoconf && ./configure --disable-utils && make |
test/explore.sh
Outdated
| set timeout 3 | ||
| spawn "$TRY" explore | ||
| send -- "PS1='# '\r" |
There was a problem hiding this comment.
This should probably not be here? Feels like rebase noise.
try
Outdated
| find "$sandbox_dir/upperdir/" -type f -o \( -type c -size 0 \) -o -type d -o -type l | ignore_changes "$ignore_file" | ||
| } | ||
| find "$sandbox_dir/upperdir/" | ignore_changes "$ignore_file" | ||
| } |
try
Outdated
| ## NB $mountpoint is the local directory to mount | ||
| ## $merger_dir is where we'll put its merger | ||
| "$UNION_HELPER" "$mountpoint" "$merger_dir" 2>>"$try_mount_log" || | ||
| "$UNION_HELPER" $mountpoint $merger_dir 2>>"$try_mount_log" || |
There was a problem hiding this comment.
Why did you remove the comment?
try
Outdated
| fi | ||
|
|
||
| # must be a directory, but not opaque---leave it! | ||
| # must be a directory, but not opaque---leave it! |
| break; | ||
|
|
||
| case FTS_DEFAULT: | ||
|
|
There was a problem hiding this comment.
These two cases can be collapsed, no?
2 participants
Added support for fifos and sockets for try. Added test cases in all-commit-cases.sh for fifos and sockets as well. Added make-socket.c for all-commit-cases.sh.