zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] portable mechanism to determine noatime
@ 2015-12-04 23:22 Baptiste Daroussin
  2015-12-05  5:15 ` Eric Cook
  0 siblings, 1 reply; 15+ messages in thread
From: Baptiste Daroussin @ 2015-12-04 23:22 UTC (permalink / raw)
  To: Zsh workers

Hello,

While updating the FreeBSD package to 5.2, I got a failure when
running the testsuite. Actually it was not a real failure are the said
test is known to not work on a noatime filesystem which is my case.

The way to test for noatime rely on /etc/mtab which we do not have

Here is a patch to use the mount command instead:
https://people.freebsd.org/~bapt/0001-Use-a-portable-mechanism-to-determine-if-the-filesys.patch

Best regards,
Bapt


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-04 23:22 [PATCH] portable mechanism to determine noatime Baptiste Daroussin
@ 2015-12-05  5:15 ` Eric Cook
  2015-12-05 18:17   ` Eric Cook
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Cook @ 2015-12-05  5:15 UTC (permalink / raw)
  To: zsh-workers

On 12/04/2015 06:22 PM, Baptiste Daroussin wrote:
> Hello,
> 
> While updating the FreeBSD package to 5.2, I got a failure when
> running the testsuite. Actually it was not a real failure are the said
> test is known to not work on a noatime filesystem which is my case.
> 
> The way to test for noatime rely on /etc/mtab which we do not have
> 
> Here is a patch to use the mount command instead:
> https://people.freebsd.org/~bapt/0001-Use-a-portable-mechanism-to-determine-if-the-filesys.patch
> 
> Best regards,
> Bapt
> 
This patch doesn't work on illumos/solaris systems.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-05  5:15 ` Eric Cook
@ 2015-12-05 18:17   ` Eric Cook
  2015-12-05 20:00     ` Matthew Martin
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Cook @ 2015-12-05 18:17 UTC (permalink / raw)
  To: zsh-workers

On 12/05/2015 12:15 AM, Eric Cook wrote:
> On 12/04/2015 06:22 PM, Baptiste Daroussin wrote:
>> Hello,
>>
>> While updating the FreeBSD package to 5.2, I got a failure when
>> running the testsuite. Actually it was not a real failure are the said
>> test is known to not work on a noatime filesystem which is my case.
>>
>> The way to test for noatime rely on /etc/mtab which we do not have
>>
>> Here is a patch to use the mount command instead:
>> https://people.freebsd.org/~bapt/0001-Use-a-portable-mechanism-to-determine-if-the-filesys.patch
>>
>> Best regards,
>> Bapt
>>
> This patch doesn't work on illumos/solaris systems.
> 
Well, it isn't just unique to illumos/solaris.
If you build zsh in a directory that is on your root partition (say you don't have a separate /home partition)
`df .' returns `/', `grep /' matches every mount point and if any mount point is mounted noatime, the final grep succeeds.
But that problem still exist with the use of /etc/mtab.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-05 18:17   ` Eric Cook
@ 2015-12-05 20:00     ` Matthew Martin
  2015-12-06  1:25       ` Oliver Kiddle
  0 siblings, 1 reply; 15+ messages in thread
From: Matthew Martin @ 2015-12-05 20:00 UTC (permalink / raw)
  To: zsh-workers

On Sat, Dec 5, 2015 at 12:17 PM, Eric Cook <llua@gmx.com> wrote:
> On 12/05/2015 12:15 AM, Eric Cook wrote:
>> On 12/04/2015 06:22 PM, Baptiste Daroussin wrote:
>>> Hello,
>>>
>>> While updating the FreeBSD package to 5.2, I got a failure when
>>> running the testsuite. Actually it was not a real failure are the said
>>> test is known to not work on a noatime filesystem which is my case.
>>>
>>> The way to test for noatime rely on /etc/mtab which we do not have
>>>
>>> Here is a patch to use the mount command instead:
>>> https://people.freebsd.org/~bapt/0001-Use-a-portable-mechanism-to-determine-if-the-filesys.patch
>>>
>>> Best regards,
>>> Bapt
>>>
>> This patch doesn't work on illumos/solaris systems.
>>
> Well, it isn't just unique to illumos/solaris.
> If you build zsh in a directory that is on your root partition (say you don't have a separate /home partition)
> `df .' returns `/', `grep /' matches every mount point and if any mount point is mounted noatime, the final grep succeeds.
> But that problem still exist with the use of /etc/mtab.

What about
mount | awk '$1 == "'"$(df . | tail -n 1 | cut -d\  -f1)"\" | grep -q noatime


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-05 20:00     ` Matthew Martin
@ 2015-12-06  1:25       ` Oliver Kiddle
  2015-12-10  8:48         ` Bart Schaefer
  0 siblings, 1 reply; 15+ messages in thread
From: Oliver Kiddle @ 2015-12-06  1:25 UTC (permalink / raw)
  To: zsh-workers

Matthew Martin wrote:
> >> This patch doesn't work on illumos/solaris systems.
> >>
> > Well, it isn't just unique to illumos/solaris.
> > If you build zsh in a directory that is on your root partition (say you don't have a separate /home partition)
> > `df .' returns `/', `grep /' matches every mount point and if any mount point is mounted noatime, the final grep succeeds.
> > But that problem still exist with the use of /etc/mtab.
>
> What about
> mount | awk '$1 == "'"$(df . | tail -n 1 | cut -d\  -f1)"\" | grep -q noatime

The problems on Solaris are:
  mount is in /sbin which is likely not to be in $path
    - we could use { mount || /sbin/mount } 2>/dev/null
  the default output from df is different.
    - use df -k .
  tail has no -n option, there is /usr/xpg4/bin but
    - use tail -1
  grep has no -q option
    - redirect to /dev/null
  mount output has the filesystem in $1 and device in $3: opposite of Linux/BSD
    - perhaps grep for the filesystem with a trailing space

Something like:
  {mount || /sbin/mount} 2>/dev/null |grep $(df -k .|tail -1 |awk '{print $1}')" .*noatime" >/dev/null

That will still break for strange characters in the mount point.
Whatever we do is probably going to be more fragile than the actual test
case it skips - [[ -N ... ]]. Perhaps we should redo the test case to
only check that -N agrees with a manual comparison after using the zstat
module.

Oliver


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-06  1:25       ` Oliver Kiddle
@ 2015-12-10  8:48         ` Bart Schaefer
  2015-12-10  8:55           ` Bart Schaefer
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2015-12-10  8:48 UTC (permalink / raw)
  To: zsh-workers

On Dec 6,  2:25am, Oliver Kiddle wrote:
}
}   mount output has the filesystem in $1 and device in $3: opposite of Linux/BSD
}     - perhaps grep for the filesystem with a trailing space

I was thinking about this.  Couple of remarks/suggestions:

 - Why don't we grep "mount" output for "noatime" FIRST, and THEN test
   whether [[ $(df .) = $(df $atimeFS) ]] ?

 - Given that, maybe we don't need to care about $1 vs. $3.

Thus:

{ df $(mount || /sbin/mount | awk '/noatime/ {print $1, $3}') | fgrep "$(df .)" } >&/dev/null


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-10  8:48         ` Bart Schaefer
@ 2015-12-10  8:55           ` Bart Schaefer
  2015-12-11  4:21             ` Oliver Kiddle
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2015-12-10  8:55 UTC (permalink / raw)
  To: zsh-workers

On Dec 10, 12:48am, Bart Schaefer wrote:
>
> { df $(mount || /sbin/mount | awk '/noatime/ {print $1, $3}') | fgrep "$(df .)" } >&/dev/null

Oops, exactly as written that runs df with no arguments if there are no
filesystems with noatime, so tweak it a little:

df ${$(mount || /sbin/mount | awk '/noatime/ {print $1, $3}'):-""}


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-10  8:55           ` Bart Schaefer
@ 2015-12-11  4:21             ` Oliver Kiddle
  2015-12-12  6:49               ` Bart Schaefer
  0 siblings, 1 reply; 15+ messages in thread
From: Oliver Kiddle @ 2015-12-11  4:21 UTC (permalink / raw)
  To: zsh-workers

Bart wrote:
> On Dec 10, 12:48am, Bart Schaefer wrote:

> - Why don't we grep "mount" output for "noatime" FIRST, and THEN test
>    whether [[ $(df .) = $(df $atimeFS) ]] 

Seems reasonable but after playing with the idea I've found it won't
work for btrfs subvolumes: they have the same device but can be mounted
with different options. I created this state:
# mount |grep /dev/sda6
/dev/sda6 on /var/lib/docker type btrfs (rw,relatime,space_cache)
/dev/sda6 on /mnt type btrfs (rw,noatime,space_cache)
df /dev/sda6 only shows one: whichever was mounted second I think.

> Oops, exactly as written that runs df with no arguments if there are no
> filesystems with noatime, so tweak it a little:
>
> df ${$(mount || /sbin/mount | awk '/noatime/ {print $1, $3}'):-""}

I needed some tweaks to make that work for me. I need -- after df
because of automounter entries such as -hosts being interpreted as
options. Braces around the two mount bits seem to be needed. And it
doesn't work to fgrep for the full output of $(df .) - just the first
word on the last line.

df -k -- ${$({mount || /sbin/mount} >&/dev/null | awk '/noatime/ {print $1, $3}'):-""} >&/dev/null |fgrep $(df -k .|tail -1|cut -d' ' -f1) >/dev/null

Oliver


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-11  4:21             ` Oliver Kiddle
@ 2015-12-12  6:49               ` Bart Schaefer
  2015-12-13  2:13                 ` Oliver Kiddle
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2015-12-12  6:49 UTC (permalink / raw)
  To: zsh-workers

On Dec 11,  5:21am, Oliver Kiddle wrote:
}
} Bart wrote:
} > df ${$(mount || /sbin/mount | awk '/noatime/ {print $1, $3}'):-""}
} 
} I needed some tweaks to make that work for me.

Indeed, sorry about that.  I just noticed I missed the subshell around
(mount || /sbin/mount) [or braces would also work].

} I need -- after df
} because of automounter entries such as -hosts being interpreted as
} options. Braces around the two mount bits seem to be needed. And it
} doesn't work to fgrep for the full output of $(df .) - just the first
} word on the last line.

Oh, duh, have to discard the first line from df with the column labels.
But the whole point is to not search for just one word:

{ df -k -- ${$({mount || /sbin/mount} | awk '/noatime/ {print $1,$3}'):-""} |
fgrep "$(df -k . | tail -1)" } >&/dev/null

Searching for a full line rather than only for one word avoids the btrfs
issue you mentioned, among other things.

Is there a reason the single >&/dev/null isn't enough to throw away all
the spurious output?  The additional two >&/dev/null that you added to
your edit only do the expected thing if multios is set (which I guess
it would be, in context, but still).


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-12  6:49               ` Bart Schaefer
@ 2015-12-13  2:13                 ` Oliver Kiddle
  2015-12-13 16:22                   ` Bart Schaefer
  2015-12-30 17:49                   ` Oliver Kiddle
  0 siblings, 2 replies; 15+ messages in thread
From: Oliver Kiddle @ 2015-12-13  2:13 UTC (permalink / raw)
  To: zsh-workers

Bart wrote:

> Searching for a full line rather than only for one word avoids the btrfs
> issue you mentioned, among other things.

I was concerned that the reported sizes mightn't be identical between
two runs of df. On testing this difference, I'm finding that the df
output isn't matching but for a different reason. df is adjusting the
column widths according to maximum lengths of fields. That ends up
varying between the one fs run and the multple fs run of df. That can be
solved by piping to tr -s ' ' or wrapping it in ${=...}.

There are also cases where after a particularly long device, df outputs
a newline so one entry is split over two lines. Solaris is clever enough
not to do this when outputting to a pipe but it occurs on at least Linux.
In practice, I don't think this matters because it will occur for both
the df runs and the last line is sufficient for fgrep.

> Is there a reason the single >&/dev/null isn't enough to throw away all
> the spurious output?  The additional two >&/dev/null that you added to
> your edit only do the expected thing if multios is set (which I guess
> it would be, in context, but still).

Each was necessary for silencing something or other but I agree that
putting everything in braces and having a single redirection is better.

Oliver


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-13  2:13                 ` Oliver Kiddle
@ 2015-12-13 16:22                   ` Bart Schaefer
  2015-12-18  2:45                     ` Oliver Kiddle
  2015-12-30 17:49                   ` Oliver Kiddle
  1 sibling, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2015-12-13 16:22 UTC (permalink / raw)
  To: zsh-workers

On Dec 13,  3:13am, Oliver Kiddle wrote:
}
} I was concerned that the reported sizes mightn't be identical between
} two runs of df.

The filesystem would have to be filling/emptying awfully quickly for
that to happen between two runs in this close proximity.

} output isn't matching but for a different reason. df is adjusting the
} column widths according to maximum lengths of fields.

Oof.  I don't suppose this and the line-folding problem could be solved
by exporting a very large value of COLUMNS ?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-13 16:22                   ` Bart Schaefer
@ 2015-12-18  2:45                     ` Oliver Kiddle
  2015-12-18  4:40                       ` Bart Schaefer
  0 siblings, 1 reply; 15+ messages in thread
From: Oliver Kiddle @ 2015-12-18  2:45 UTC (permalink / raw)
  To: zsh-workers

On 13 Dec, Bart wrote:
> } output isn't matching but for a different reason. df is adjusting the
> } column widths according to maximum lengths of fields.
> 
> Oof.  I don't suppose this and the line-folding problem could be solved
> by exporting a very large value of COLUMNS ?

No. df ignores that. I'm not sure I've ever come across external
programs as opposed to shell scripts that honour a value in $COLUMNS.

Oliver


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-18  2:45                     ` Oliver Kiddle
@ 2015-12-18  4:40                       ` Bart Schaefer
  0 siblings, 0 replies; 15+ messages in thread
From: Bart Schaefer @ 2015-12-18  4:40 UTC (permalink / raw)
  To: zsh-workers

On Dec 18,  3:45am, Oliver Kiddle wrote:
} Subject: Re: [PATCH] portable mechanism to determine noatime
}
} On 13 Dec, Bart wrote:
} > Oof.  I don't suppose this and the line-folding problem could be solved
} > by exporting a very large value of COLUMNS ?
} 
} No. df ignores that. I'm not sure I've ever come across external
} programs as opposed to shell scripts that honour a value in $COLUMNS.

GNU "ls -C" honors COLUMNS when the output is not a terminal (e.g., is
a pipe), so it was at least worth a shot.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-13  2:13                 ` Oliver Kiddle
  2015-12-13 16:22                   ` Bart Schaefer
@ 2015-12-30 17:49                   ` Oliver Kiddle
  2015-12-30 22:43                     ` Bart Schaefer
  1 sibling, 1 reply; 15+ messages in thread
From: Oliver Kiddle @ 2015-12-30 17:49 UTC (permalink / raw)
  To: zsh-workers

On 13 Dec, I wrote:
> df is adjusting the
> column widths according to maximum lengths of fields. That ends up
> varying between the one fs run and the multple fs run of df. That can be
> solved by piping to tr -s ' ' or wrapping it in ${=...}.

So is there any advance on the patch below or shall I commit it in
this form?

Oliver

diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
index 9e13696..0b4608a 100644
--- a/Test/C02cond.ztst
+++ b/Test/C02cond.ztst
@@ -154,7 +154,7 @@
     ZTST_skip="[[ -N file ]] not supported on Cygwin"
   elif (( isnfs )); then
     ZTST_skip="[[ -N file ]] not supported with NFS"
-  elif test -f /etc/mtab && { grep $(df . 2>/dev/null| tail -n1 | awk '{print $1}') /etc/mtab | grep -q noatime; }; then
+  elif { df -k -- ${$({mount || /sbin/mount} | awk '/noatime/ {print $1,$3}'):-""} | tr -s ' ' | fgrep "$(df -k . | tail -1 | tr -s ' ')" } >&/dev/null; then
     ZTST_skip="[[ -N file ]] not supported with noatime file system"
   else
     [[ -N $newnewnew && ! -N $unmodified ]]


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] portable mechanism to determine noatime
  2015-12-30 17:49                   ` Oliver Kiddle
@ 2015-12-30 22:43                     ` Bart Schaefer
  0 siblings, 0 replies; 15+ messages in thread
From: Bart Schaefer @ 2015-12-30 22:43 UTC (permalink / raw)
  To: zsh-workers

On Dec 30,  6:49pm, Oliver Kiddle wrote:
}
} So is there any advance on the patch below or shall I commit it in
} this form?

I don't think we improved beyond this, so go for it.  Field testing is
always the best indicator.


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-12-30 22:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-04 23:22 [PATCH] portable mechanism to determine noatime Baptiste Daroussin
2015-12-05  5:15 ` Eric Cook
2015-12-05 18:17   ` Eric Cook
2015-12-05 20:00     ` Matthew Martin
2015-12-06  1:25       ` Oliver Kiddle
2015-12-10  8:48         ` Bart Schaefer
2015-12-10  8:55           ` Bart Schaefer
2015-12-11  4:21             ` Oliver Kiddle
2015-12-12  6:49               ` Bart Schaefer
2015-12-13  2:13                 ` Oliver Kiddle
2015-12-13 16:22                   ` Bart Schaefer
2015-12-18  2:45                     ` Oliver Kiddle
2015-12-18  4:40                       ` Bart Schaefer
2015-12-30 17:49                   ` Oliver Kiddle
2015-12-30 22:43                     ` Bart Schaefer

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).