Re: [PATCH v3 1/1] t0027: tests are not expensive; remove t0025
Ok for me, thanks for helping out. > Am 10.05.2017 um 17:52 schrieb Johannes Schindelin > : > > Hi, > >> On Wed, 10 May 2017, tbo...@web.de wrote: >> >> From: Torsten Bögershausen >> >> The purpose of t0027 is to test all CRLF related conversions at "git >> checkout" >> and "git add". >> >> Running t0027 under Git for Windows takes 3-4 minutes, so the whole script >> had >> been marked as "EXPENSIVE". >> >> The source code for "Git for Windows" overrides this since 2014: >> "t0027 is marked expensive, but really, for MinGW we want to run these >> tests always." >> >> Recent "stress" tests show that t0025 if flaky, reported by Lars Schneider, >> larsxschnei...@gmail.com >> >> All tests in t0025 are covered by t0027 already, so that t0025 can be >> retired. >> t0027 takes less than 14 seconds under Linux, and 63 seconds under Mac Os X, >> and this is more or less the same with a SSD or a spinning disk. >> >> Acked-by: Johannes Schindelin >> Signed-off-by: Torsten Bögershausen > > This is still formatted very awkwardly. How about this instead (I fixed > the formatting, reworded a little here and there, and fixed the order of > the footers)? > > -- snipsnap -- > From: Torsten Bögershausen > > The purpose of t0027 is to test all CRLF related conversions at "git > checkout" and "git add". Running t0027 under Git for Windows takes 3-4 > minutes, so the whole script had been marked as "EXPENSIVE". > > However, the "Git for Windows" fork overrides this since 2014: "t0027 > is marked expensive, but really, for MinGW we want to run these tests > always." > > The test seems not to be expensive on other platforms at all: it takes > less than 14 seconds under Linux, and 63 seconds under Mac Os X, and > this is more or less the same with a SSD or a spinning disk. > > So let's drop the "EXPENSIVE" prereq. > > While at it, retire t0025: Recent "stress" tests show that t0025 if > flaky, reported by Lars Schneider , but all > tests in t0025 are covered by t0027 already. > > Signed-off-by: Torsten Bögershausen > Acked-by: Johannes Schindelin
Re: [PATCH] t7063: work around FreeBSD's lazy mtime update feature
> Am 30.07.2016 um 15:20 schrieb Nguyễn Thái Ngọc Duy : > > Let's start with the commit message of [1] from freebsd.git [2] > >Sync timestamp changes for inodes of special files to disk as late >as possible (when the inode is reclaimed). Temporarily only do >this if option UFS_LAZYMOD configured and softupdates aren't >enabled. UFS_LAZYMOD is intentionally left out of >/sys/conf/options. > >This is mainly to avoid almost useless disk i/o on battery powered >machines. It's silly to write to disk (on the next sync or when >the inode becomes inactive) just because someone hit a key or >something wrote to the screen or /dev/null. > >PR: 5577 [3] > > The short version of that, in the context of t7063, is that when a > directory is updated, its mtime may be updated later, not > immediately. This can be shown with a simple command sequence > >date; sleep 1; touch abc; rm abc; sleep 10; ls -lTd . > > One would expect that the date shown in `ls` would be one second from > `date`, but it's 10 seconds later. If we put another `ls -lTd .` in > front of `sleep 10`, then the date of the last `ls` comes as > expected. The first `ls` somehow forces mtime to be updated. > > t7063 is really sensitive to directory mtime. When mtime is too "new", > git code suspects racy timestamps and will not trigger the shortcut in > untracked cache, in t7063.26 (or t7063.25 before this patch) and > eventually be detected in t7063.28 > > We have two options thanks to this special FreeBSD feature: > > 1) Stop supporting untracked cache on FreeBSD. Skip t7063 entirely > when running on FreeBSD > > 2) Work around this problem (using the same 'ls' trick) and continue > to support untracked cache on FreeBSD > > I initially wanted to go with 1) because I didn't know the exact > nature of this feature and feared that it would make untracked cache > work unreliably, using the cached version when it should not. > > Since the behavior of this thing is clearer now. The picture is not > that bad. If this indeed happens often, untracked cache would assume > racy condition more often and _fall back_ to non-untracked cache code > paths. Which means it may be less effective, but it will not show > wrong things. > > This patch goes with option 2. > > PS. For those who want to look further in FreeBSD source code, this > flag is now called IN_LAZYMOD. I can see it's effective in ext2 and > ufs. zfs is questionable. > > [1] 660e6408e6df99a20dacb070c5e7f9739efdf96d > [2] git://github.com/freebsd/freebsd.git > [3] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=5577 > > Reported-by: Eric Wong > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > This is only of those commits that commit messages are more important > than the patch itself. One of the good notes about directory mtime, > if we start to use it in more places in git. > > t/t7063-status-untracked-cache.sh | 4 > t/test-lib.sh | 6 ++ > 2 files changed, 10 insertions(+) > > diff --git a/t/t7063-status-untracked-cache.sh > b/t/t7063-status-untracked-cache.sh > index a971884..08fc586 100755 > --- a/t/t7063-status-untracked-cache.sh > +++ b/t/t7063-status-untracked-cache.sh > @@ -419,6 +419,10 @@ test_expect_success 'create/modify files, some of which > are gitignored' ' >rm base > ' > > +test_expect_success FREEBSD 'Work around lazy mtime update' ' > +ls -ld . >/dev/null > +' > + the term FREEBSD may be too generic to point out a single feature in an OS distributution. Following your investigations, it may even be possible that other systems adapt this "feature"? How about LAZY_DIR_MTIME_UPDATE (or similar) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/10] pkt-line: add packet_flush_gentle()
> Am 29.07.2016 um 20:37 schrieb larsxschnei...@gmail.com:
>
> From: Lars Schneider
>
> packet_flush() would die in case of a write error even though for some callers
> an error would be acceptable.
What happens if there is a write error ?
Basically the protocol is out of synch.
Lenght information is mixed up with payload, or the other way
around.
It may be, that the consequences of a write error are acceptable,
because a filter is allowed to fail.
What is not acceptable is a "broken" protocol.
The consequence schould be to close the fd and tear down all
resources. connected to it.
In our case to terminate the external filter daemon in some way,
and to never use this instance again.
> Add packet_flush_gentle() which writes a pkt-line
> flush packet and returns `0` for success and `1` for failure.
>
> Signed-off-by: Lars Schneider
> ---
> pkt-line.c | 6 ++
> pkt-line.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 6fae508..1728690 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -91,6 +91,12 @@ void packet_flush(int fd)
> write_or_die(fd, "", 4);
> }
>
> +int packet_flush_gentle(int fd)
> +{
> +packet_trace("", 4, 1);
> +return !write_or_whine_pipe(fd, "", 4, "flush packet");
> +}
> +
> void packet_buf_flush(struct strbuf *buf)
> {
> packet_trace("", 4, 1);
> diff --git a/pkt-line.h b/pkt-line.h
> index 02dcced..3953c98 100644
> --- a/pkt-line.h
> +++ b/pkt-line.h
> @@ -23,6 +23,7 @@ void packet_flush(int fd);
> void packet_write(int fd, const char *fmt, ...) __attribute__((format
> (printf, 2, 3)));
> void packet_buf_flush(struct strbuf *buf);
> void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
> __attribute__((format (printf, 2, 3)));
> +int packet_flush_gentle(int fd);
> int direct_packet_write(int fd, char *buf, size_t size, int gentle);
> int direct_packet_write_data(int fd, const char *buf, size_t size, int
> gentle);
>
> --
> 2.9.0
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
