| Message ID | 215801b02aceeed1e0f6313679c567a914ad5dd8.1584625896.git.congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix test failure with busybox | expand |
On Thu, Mar 19, 2020 at 09:00:06PM +0700, Đoàn Trần Công Danh wrote: > Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime > update feature, 2016-08-03), we started to use ls as a trick to update > directory's mtime. > > However, `-ls` flag isn't required by POSIX's find(1), and > busybox(1) doesn't implement it. > > Use an equivalence `-exec ls -dils {} +` instead. Makes sense. I wonder if we need all of "-dils", but it's not clear to me which syscalls actually trigger the FreeBSD lazy-update behavior. I guess probably it's stat()ing the directory, so "ls -ld" would be sufficient (and that's implied by the examples in 6b7728db81). But I doubt the extra options would create a portability problem, so I think it's fine either way. -Peff
Jeff King <peff@peff.net> writes: > On Thu, Mar 19, 2020 at 09:00:06PM +0700, Đoàn Trần Công Danh wrote: > >> Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime >> update feature, 2016-08-03), we started to use ls as a trick to update >> directory's mtime. >> >> However, `-ls` flag isn't required by POSIX's find(1), and >> busybox(1) doesn't implement it. >> >> Use an equivalence `-exec ls -dils {} +` instead. > > Makes sense. I wonder if we need all of "-dils", but it's not clear to > me which syscalls actually trigger the FreeBSD lazy-update behavior. I > guess probably it's stat()ing the directory, so "ls -ld" would be > sufficient (and that's implied by the examples in 6b7728db81). > > But I doubt the extra options would create a portability problem, so I > think it's fine either way. Thanks. I too wondered if -dils is really needed (POSIX of course have all of them, but we have to deal with non-POSIX systems, too, and I am not sure how things like "-i" works there). s/equivalence/equivalent/; perhaps?
On 2020-03-19 15:16:09-0700, Junio C Hamano <gitster@pobox.com> wrote: > Jeff King <peff@peff.net> writes: > > > On Thu, Mar 19, 2020 at 09:00:06PM +0700, Đoàn Trần Công Danh wrote: > > > >> Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime > >> update feature, 2016-08-03), we started to use ls as a trick to update > >> directory's mtime. > >> > >> However, `-ls` flag isn't required by POSIX's find(1), and > >> busybox(1) doesn't implement it. > >> > >> Use an equivalence `-exec ls -dils {} +` instead. > > > > Makes sense. I wonder if we need all of "-dils", but it's not clear to From the original commit message, I think whichever flags that call stat(2) would be do it. It's `-d` (to check is_directory), and `-i` for inode number. This make make wonder, will it be enough to just use: find . -type d >/dev/null > > me which syscalls actually trigger the FreeBSD lazy-update behavior. I > > guess probably it's stat()ing the directory, so "ls -ld" would be > > sufficient (and that's implied by the examples in 6b7728db81). > > > > But I doubt the extra options would create a portability problem, so I > > think it's fine either way. > > Thanks. I too wondered if -dils is really needed (POSIX of course > have all of them, but we have to deal with non-POSIX systems, too, > and I am not sure how things like "-i" works there). I think "-i" asks for stat(2) to get inode number, which will ask FreeBSD sync mtime. > > s/equivalence/equivalent/; perhaps? Will do, I've never correctly used -ence and -ent pairs of words.
On 2020-03-20 08:41:42+0700, Danh Doan <congdanhqx@gmail.com> wrote: > On 2020-03-19 15:16:09-0700, Junio C Hamano <gitster@pobox.com> wrote: > > Jeff King <peff@peff.net> writes: > > > > > On Thu, Mar 19, 2020 at 09:00:06PM +0700, Đoàn Trần Công Danh wrote: > > > > > >> Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime > > >> update feature, 2016-08-03), we started to use ls as a trick to update > > >> directory's mtime. > > >> > > >> However, `-ls` flag isn't required by POSIX's find(1), and > > >> busybox(1) doesn't implement it. > > >> > > >> Use an equivalence `-exec ls -dils {} +` instead. > > > > > > Makes sense. I wonder if we need all of "-dils", but it's not clear to > > From the original commit message, I think whichever flags that call > stat(2) would be do it. It's `-d` (to check is_directory), and `-i` > for inode number. > > This make make wonder, will it be enough to just use: > > find . -type d >/dev/null From the conversation in: xmqqmvktakvt.fsf@gitster.mtv.corp.google.com I think find . -type d would trigger enough lstat(2), and `-ls` was added for separated stat(2). I guess either `-exec ls -d` or `-exec ls -i` will be enough. > > > > me which syscalls actually trigger the FreeBSD lazy-update behavior. I > > > guess probably it's stat()ing the directory, so "ls -ld" would be > > > sufficient (and that's implied by the examples in 6b7728db81). > > > > > > But I doubt the extra options would create a portability problem, so I > > > think it's fine either way. > > > > Thanks. I too wondered if -dils is really needed (POSIX of course > > have all of them, but we have to deal with non-POSIX systems, too, > > and I am not sure how things like "-i" works there). > > I think "-i" asks for stat(2) to get inode number, > which will ask FreeBSD sync mtime. > > > > s/equivalence/equivalent/; perhaps? > > Will do, I've never correctly used -ence and -ent pairs of words.
On Fri, Mar 20, 2020 at 08:41:42AM +0700, Danh Doan wrote: > On 2020-03-19 15:16:09-0700, Junio C Hamano <gitster@pobox.com> wrote: > > Jeff King <peff@peff.net> writes: > > > > > On Thu, Mar 19, 2020 at 09:00:06PM +0700, Đoàn Trần Công Danh wrote: > > > > > >> Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime > > >> update feature, 2016-08-03), we started to use ls as a trick to update > > >> directory's mtime. > > >> > > >> However, `-ls` flag isn't required by POSIX's find(1), and > > >> busybox(1) doesn't implement it. > > >> > > >> Use an equivalence `-exec ls -dils {} +` instead. > > > > > > Makes sense. I wonder if we need all of "-dils", but it's not clear to > > From the original commit message, I think whichever flags that call > stat(2) would be do it. It's `-d` (to check is_directory), and `-i` > for inode number. > > This make make wonder, will it be enough to just use: > > find . -type d >/dev/null Perhaps we can get a friendly FreeBSD developer (cc'd) to run a quick test for us. Ed, the question is whether: diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 190ae149cf..6791c6b95a 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -18,7 +18,7 @@ GIT_FORCE_UNTRACKED_CACHE=true export GIT_FORCE_UNTRACKED_CACHE sync_mtime () { - find . -type d -ls >/dev/null + find . -type d >/dev/null } avoid_racy() { lets t7063 consistently pass on FreeBSD. -Peff
On 2020-03-20 01:37:30-0400, Jeff King <peff@peff.net> wrote: > On Fri, Mar 20, 2020 at 08:41:42AM +0700, Danh Doan wrote: > > > On 2020-03-19 15:16:09-0700, Junio C Hamano <gitster@pobox.com> wrote: > > > Jeff King <peff@peff.net> writes: > > > > > > > On Thu, Mar 19, 2020 at 09:00:06PM +0700, Đoàn Trần Công Danh wrote: > > > > > > > >> Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime > > > >> update feature, 2016-08-03), we started to use ls as a trick to update > > > >> directory's mtime. > > > >> > > > >> However, `-ls` flag isn't required by POSIX's find(1), and > > > >> busybox(1) doesn't implement it. > > > >> > > > >> Use an equivalence `-exec ls -dils {} +` instead. > > > > > > > > Makes sense. I wonder if we need all of "-dils", but it's not clear to > > > > From the original commit message, I think whichever flags that call > > stat(2) would be do it. It's `-d` (to check is_directory), and `-i` > > for inode number. > > > > This make make wonder, will it be enough to just use: > > > > find . -type d >/dev/null > > Perhaps we can get a friendly FreeBSD developer (cc'd) to run a quick > test for us. > > Ed, the question is whether: > > diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh > index 190ae149cf..6791c6b95a 100755 > --- a/t/t7063-status-untracked-cache.sh > +++ b/t/t7063-status-untracked-cache.sh > @@ -18,7 +18,7 @@ GIT_FORCE_UNTRACKED_CACHE=true > export GIT_FORCE_UNTRACKED_CACHE > > sync_mtime () { > - find . -type d -ls >/dev/null > + find . -type d >/dev/null > } > > avoid_racy() { > > lets t7063 consistently pass on FreeBSD. I've tried myself with a FreeBSD VM which stays on top of an HDD, t7063 consistently pass for 1000 run. I guess it should be fine I'll resend with this while waiting for Ed's response.
On Sun, Mar 22, 2020 at 07:37:03AM +0700, Danh Doan wrote: > > sync_mtime () { > > - find . -type d -ls >/dev/null > > + find . -type d >/dev/null > > } > > > > avoid_racy() { > > > > lets t7063 consistently pass on FreeBSD. > > I've tried myself with a FreeBSD VM which stays on top of an HDD, > t7063 consistently pass for 1000 run. > > I guess it should be fine > > I'll resend with this while waiting for Ed's response. Thanks, that's good enough for me. I just didn't want us committing something without _somebody_ trying it on FreeBSD (and I was too lazy to install a VM myself). -Peff
diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 190ae149cf..c2731d445a 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -18,7 +18,7 @@ GIT_FORCE_UNTRACKED_CACHE=true export GIT_FORCE_UNTRACKED_CACHE sync_mtime () { - find . -type d -ls >/dev/null + find . -type d -exec ls -dils {} + >/dev/null } avoid_racy() {
Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime update feature, 2016-08-03), we started to use ls as a trick to update directory's mtime. However, `-ls` flag isn't required by POSIX's find(1), and busybox(1) doesn't implement it. Use an equivalence `-exec ls -dils {} +` instead. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- t/t7063-status-untracked-cache.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)