zsh-workers
 help / color / mirror / code / Atom feed
* Issues with fcntl() history file locking
@ 2019-02-27 18:30 Philippe Troin
  2019-02-27 21:27 ` Bart Schaefer
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Troin @ 2019-02-27 18:30 UTC (permalink / raw)
  To: Zsh hackers list

Hi,

I've been using zsh with share_history for many years and never had any
real issues on several networks where my home directory is mounted over
NFS.  Recently, it's been giving me trouble, maybe when I bumped up my
history file size to 10k entries.

Terminal 1 on host1:                 Terminal 2 on host 2:
host1% echo 1 2 3<ENTER>             host2%
1 2 3
host1%
                                     <ENTER>
                                     host2% <UP>
                                     host2% unrelated command

I'd expect that on pressing UP on host2, my last host1 command would
show up.  It does most of the times, but not reliably enough to make me
completely happy :-)

I then discovered hist_fcntl_lock, which I had not ever set, and turned
it on.  It didn't improve anything.

After a bit of stracing, and subsequent reading of the code, I found
out that the the history file is opened and closed many times during
the history file manipulation, while the lock is maintained on one of
the open descriptors.
Unfortunately, POSIX states that the fcntl() lock will be released upon
the closing the first descriptor to the file.  Quoth 'man -s 3p fcntl':

   All locks associated with a file for a given process
   shall be removed when a file descriptor for that file is closed by
   that process or the process holding that file descriptor terminates.

The key word here is "a", in "a file descriptor ... is closed".
Don't you love standardese?

If you look at Src/hist.c, you'll see that locks are sprinkled
everywhere and both readhistfile() and writehistfile() open the history
file and are cross-recursive.
We can totally end up in a case where:
 * flockhistfile opens and puts a write lock on the history file.
 * writehistfile opens a new fd to the same file
 * history needs to be merged/trimmed or whatever else leading to a
   recursive call to...
 * readhistfile, which opens another fd to the same file, and closes
   it, at which point the lock is lost.
 * writehistfile writes the history file without lock
 * ...

Now I'm not sure if that's what's causing my mysterious shared history
lapses, but fixing that problem shouldn't hurt.

After contemplating Src/hist.c for a bit, it won't be a trivial fix.
I see two ways:  the right and hard way, and the easy messy way.

The right and hard way is to have the various calls to open() the
history file to actually use the flock_fd lock file descriptor (and not
close it when done with it, leaving that to unlockhistfile()).
I think we can open the descriptor in flockhistfile() with O_APPEND
since I haven't spotted any location where we do not write at the end
of the file.  O_APPEND can't hurt if we don't write in the middle of
the file.
That leaves the issue of truncating the file when needed.  We cannot
open(...O_TRUNC...) for the same reason:  we will need ftruncate(),
which we've avoided all these years :-) and probably an autoconf
feature test.
We may also have to keep track (or reset) the seek pointer depending on
the sequencing of the calls, I haven't fully investigated the need for
that.
The whole thing will certainly not improve the clarity of the code :-/

The easy messy way is to keep track of all the open descriptors to the
history file in a global variable, and delaying the actual close until
unlockhistfile() is called.
While it's conceptually fugly, it's arguably easier to maintain and
understand.

Any opinions?

Phil.

===
Full history options and variables
% echo $ZSH_VERSION 
   5.6.2
% setopt | grep hist
   noappendhistory       off
   nobanghist            on
   cshjunkiehistory      off
   extendedhistory       on
   histallowclobber      off
   nohistbeep            off
   histexpiredupsfirst   on
   histfcntllock         on
   histfindnodups        off
   histignorealldups     off
   histignoredups        on
   histignorespace       off
   histlexwords          off
   histnofunctions       off
   histnostore           off
   histreduceblanks      on
   nohistsavebycopy      off
   histsavenodups        off
   histsubstpattern      off
   histverify            off
   incappendhistory      off
   incappendhistorytime  off
   sharehistory          on
% set | grep -i hist
   HISTCHARS='!^#'
   HISTCMD=10264
   HISTFILE=/home/phil/.zhistory
   HISTSIZE=40960
   SAVEHIST=10240



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

* Re: Issues with fcntl() history file locking
  2019-02-27 18:30 Issues with fcntl() history file locking Philippe Troin
@ 2019-02-27 21:27 ` Bart Schaefer
  2019-02-28  6:36   ` Philippe Troin
  0 siblings, 1 reply; 11+ messages in thread
From: Bart Schaefer @ 2019-02-27 21:27 UTC (permalink / raw)
  To: Philippe Troin; +Cc: Zsh hackers list

On Wed, Feb 27, 2019 at 10:31 AM Philippe Troin <phil@fifi.org> wrote:
>
> I've been using zsh with share_history for many years and never had any
> real issues on several networks where my home directory is mounted over
> NFS.  Recently, it's been giving me trouble, maybe when I bumped up my
> history file size to 10k entries.
>
> I then discovered hist_fcntl_lock, which I had not ever set, and turned
> it on.  It didn't improve anything.

Well, it wouldn't ... in fact it would likely make things worse.
flock() historically doesn't work reliably over NFS, and if you turn
that option on you are disabling the symlink-based file locking that
is usually more NFS-friendly.  We used to do both kinds of locking
when hist_fcntl_lock, but workers/32580 reverted to using only one
kind ... I forget why I was asked to do that, probably something not
working as fast as was desired.

> Unfortunately, POSIX states that the fcntl() lock will be released upon
> the closing the first descriptor to the file.  [...and thus...]
>
>  * writehistfile writes the history file without lock

If that were the problem, you'd be likely to see corrupted entries
(the read stopping somewhere in the middle of what's being written) or
problems when both shells were writing to the file, which would also
likely manifest as corrupted entries.

Do the entries from terminal 1 NEVER show up in the file?  Are they in
the file but never show up in the history of terminal 2?  Or are they
just slow to arrive in terminal 2?

I'd be more inclined to suspect async NFS issues rather than locking.
Have you strace'd both processes to see when writes v. reads are
happening?

> The right and hard way is to have the various calls to open() the
> history file to actually use the flock_fd lock file descriptor (and not
> close it when done with it, leaving that to unlockhistfile()).
>
> The easy messy way is to keep track of all the open descriptors to the
> history file in a global variable, and delaying the actual close until
> unlockhistfile() is called.

If this actually turns out to be necessary, the second way is more
similar to how we handle descriptors in other parts of the shell.

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

* Re: Issues with fcntl() history file locking
  2019-02-27 21:27 ` Bart Schaefer
@ 2019-02-28  6:36   ` Philippe Troin
  2019-03-09  0:53     ` Philippe Troin
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Troin @ 2019-02-28  6:36 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 2019-02-27 at 13:27 -0800, Bart Schaefer wrote:
> On Wed, Feb 27, 2019 at 10:31 AM Philippe Troin <phil@fifi.org>
> wrote:
> > I've been using zsh with share_history for many years and never had
> > any
> > real issues on several networks where my home directory is mounted
> > over
> > NFS.  Recently, it's been giving me trouble, maybe when I bumped up
> > my
> > history file size to 10k entries.
> > 
> > I then discovered hist_fcntl_lock, which I had not ever set, and
> > turned
> > it on.  It didn't improve anything.
> 
> Well, it wouldn't ... in fact it would likely make things worse.
> flock() historically doesn't work reliably over NFS, and if you turn
> that option on you are disabling the symlink-based file locking that
> is usually more NFS-friendly.  We used to do both kinds of locking
> when hist_fcntl_lock, but workers/32580 reverted to using only one
> kind ... I forget why I was asked to do that, probably something not
> working as fast as was desired.

Not necessarily worse.
While you're right that (BSD) flock() never worked correctly on NFS,
that is not the case with POSIX fcntl() locks.  Zsh uses the later even
though the zsh functions are named flock*.
Also locking the file with fcntl clears the NFS attribute cache for
that inode, making sure that you get the latest data.

> > Unfortunately, POSIX states that the fcntl() lock will be released
> > upon
> > the closing the first descriptor to the file.  [...and thus...]
> > 
> >  * writehistfile writes the history file without lock
> 
> If that were the problem, you'd be likely to see corrupted entries
> (the read stopping somewhere in the middle of what's being written)
> or
> problems when both shells were writing to the file, which would also
> likely manifest as corrupted entries.
> 
> Do the entries from terminal 1 NEVER show up in the file?  Are they
> in
> the file but never show up in the history of terminal 2?  Or are they
> just slow to arrive in terminal 2?
> 
> I'd be more inclined to suspect async NFS issues rather than locking.
> Have you strace'd both processes to see when writes v. reads are
> happening?

The history file never gets corrupted.  What I'm experiencing is loss
of sync for a while.  New commands on host1 never seem to appear (or
take a long time to appear) on host2.
Given this happens randomly, it's hard to catch zsh in the act.

> > The right and hard way is to have the various calls to open() the
> > history file to actually use the flock_fd lock file descriptor (and
> > not
> > close it when done with it, leaving that to unlockhistfile()).
> > 
> > The easy messy way is to keep track of all the open descriptors to
> > the
> > history file in a global variable, and delaying the actual close
> > until
> > unlockhistfile() is called.
> 
> If this actually turns out to be necessary, the second way is more
> similar to how we handle descriptors in other parts of the shell.

I'll do further experiments.

This is my current hunch:  everything is swell as long as lines are
appended to the history file.  But, when one host decides it's time to
trim the history file is when stuff hits the fan.  If someone had an
idea on how to force zsh to trim history reliably, I'm all ears.

Phil.


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

* Re: Issues with fcntl() history file locking
  2019-02-28  6:36   ` Philippe Troin
@ 2019-03-09  0:53     ` Philippe Troin
  2019-03-09  0:54       ` [PATCH 1/3] Move readhistfile() after flockhistfile() Philippe Troin
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Troin @ 2019-03-09  0:53 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 2019-02-27 at 22:36 -0800, Philippe Troin wrote:
> On Wed, 2019-02-27 at 13:27 -0800, Bart Schaefer wrote:
> > On Wed, Feb 27, 2019 at 10:31 AM Philippe Troin <phil@fifi.org>
> > wrote:
> > > 
> > > The right and hard way is to have the various calls to open() the
> > > history file to actually use the flock_fd lock file descriptor
> > > (and not close it when done with it, leaving that to
> > > unlockhistfile()).
> > > 
> > > The easy messy way is to keep track of all the open descriptors
> > > to the history file in a global variable, and delaying the actual
> > > close until unlockhistfile() is called.
> > 
> > If this actually turns out to be necessary, the second way is more
> > similar to how we handle descriptors in other parts of the shell.
> 
> I'll do further experiments.
> 
> This is my current hunch:  everything is swell as long as lines are
> appended to the history file.  But, when one host decides it's time
> to
> trim the history file is when stuff hits the fan.  If someone had an
> idea on how to force zsh to trim history reliably, I'm all ears.

So, I've implemented what I described (delaying closing the file
descriptors and handles until unlock time).
The rough patches will follow shortly.

But, it doesn't help the problem at all:  I've been running 5.7.1 +
that patch on a small network, and I ran into the same issue again :-/
Back to the drawing board...
It's something else that's happening.

I'll let you decide if the patches are worth inclusion or not.
Since it doesn't fix any actual issues, probably not, even though this
is supposed to be more correct to avoid history corruption on network
filesystems when hist_fcntl_lock is active.

Phil.


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

* [PATCH 1/3] Move readhistfile() after flockhistfile().
  2019-03-09  0:53     ` Philippe Troin
@ 2019-03-09  0:54       ` Philippe Troin
  2019-03-09  0:54         ` [PATCH 2/3] Factorize the code that unlock the fcntl() lock into funlockhistfile() Philippe Troin
                           ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Philippe Troin @ 2019-03-09  0:54 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Philippe Troin

---
 Src/hist.c | 96 +++++++++++++++++++++++++++---------------------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/Src/hist.c b/Src/hist.c
index f7e53de74..4e1f24afe 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2525,6 +2525,54 @@ readhistline(int start, char **bufp, int *bufsiz, FILE *in)
     return 0;
 }
 
+#ifdef HAVE_FCNTL_H
+static int flock_fd = -1;
+
+/*
+ * Lock file using fcntl().  Return 0 on success, 1 on failure of
+ * locking mechanism, 2 on permanent failure (e.g. permission).
+ */
+
+static int
+flockhistfile(char *fn, int keep_trying)
+{
+    struct flock lck;
+    long sleep_us = 0x10000; /* about 67 ms */
+    time_t end_time;
+
+    if (flock_fd >= 0)
+	return 0; /* already locked */
+
+    if ((flock_fd = open(unmeta(fn), O_RDWR | O_NOCTTY)) < 0)
+	return errno == ENOENT ? 0 : 2; /* "successfully" locked missing file */
+
+    lck.l_type = F_WRLCK;
+    lck.l_whence = SEEK_SET;
+    lck.l_start = 0;
+    lck.l_len = 0;  /* lock the whole file */
+
+    /*
+     * Timeout is ten seconds.
+     */
+    end_time = time(NULL) + (time_t)10;
+    while (fcntl(flock_fd, F_SETLKW, &lck) == -1) {
+	if (!keep_trying || time(NULL) >= end_time ||
+	    /*
+	     * Randomise wait to minimise clashes with shells exiting at
+	     * the same time.
+	     */
+	    !zsleep_random(sleep_us, end_time)) {
+	    close(flock_fd);
+	    flock_fd = -1;
+	    return 1;
+	}
+	sleep_us <<= 1;
+    }
+
+    return 0;
+}
+#endif
+
 /**/
 void
 readhistfile(char *fn, int err, int readflags)
@@ -2721,54 +2769,6 @@ readhistfile(char *fn, int err, int readflags)
 	zleentry(ZLE_CMD_SET_HIST_LINE, curhist);
 }
 
-#ifdef HAVE_FCNTL_H
-static int flock_fd = -1;
-
-/*
- * Lock file using fcntl().  Return 0 on success, 1 on failure of
- * locking mechanism, 2 on permanent failure (e.g. permission).
- */
-
-static int
-flockhistfile(char *fn, int keep_trying)
-{
-    struct flock lck;
-    long sleep_us = 0x10000; /* about 67 ms */
-    time_t end_time;
-
-    if (flock_fd >= 0)
-	return 0; /* already locked */
-
-    if ((flock_fd = open(unmeta(fn), O_RDWR | O_NOCTTY)) < 0)
-	return errno == ENOENT ? 0 : 2; /* "successfully" locked missing file */
-
-    lck.l_type = F_WRLCK;
-    lck.l_whence = SEEK_SET;
-    lck.l_start = 0;
-    lck.l_len = 0;  /* lock the whole file */
-
-    /*
-     * Timeout is ten seconds.
-     */
-    end_time = time(NULL) + (time_t)10;
-    while (fcntl(flock_fd, F_SETLKW, &lck) == -1) {
-	if (!keep_trying || time(NULL) >= end_time ||
-	    /*
-	     * Randomise wait to minimise clashes with shells exiting at
-	     * the same time.
-	     */
-	    !zsleep_random(sleep_us, end_time)) {
-	    close(flock_fd);
-	    flock_fd = -1;
-	    return 1;
-	}
-	sleep_us <<= 1;
-    }
-
-    return 0;
-}
-#endif
-
 /**/
 void
 savehistfile(char *fn, int err, int writeflags)
-- 
2.20.1


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

* [PATCH 2/3] Factorize the code that unlock the fcntl() lock into funlockhistfile().
  2019-03-09  0:54       ` [PATCH 1/3] Move readhistfile() after flockhistfile() Philippe Troin
@ 2019-03-09  0:54         ` Philippe Troin
  2019-03-09  0:54         ` [PATCH 3/3] Delay closing the history file until the fcntl-lock is released Philippe Troin
       [not found]         ` <CAH+w=7aUD11M_GYy-FOC5MPGpGXb+o9O_q855OTC32fnSnpshQ@mail.gmail.com>
  2 siblings, 0 replies; 11+ messages in thread
From: Philippe Troin @ 2019-03-09  0:54 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Philippe Troin

---
 Src/hist.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Src/hist.c b/Src/hist.c
index 4e1f24afe..981316674 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2571,6 +2571,15 @@ flockhistfile(char *fn, int keep_trying)
 
     return 0;
 }
+
+static void
+funlockhistfile()
+{
+    if (flock_fd >= 0) {
+	close(flock_fd);
+	flock_fd = -1;
+    }
+}
 #endif
 
 /**/
@@ -2951,10 +2960,7 @@ savehistfile(char *fn, int err, int writeflags)
 		} else {
 		    /* We renamed over the locked HISTFILE, so close fd.
 		     * If we do more writing, we'll get a lock then. */
-		    if (flock_fd >= 0) {
-			close(flock_fd);
-			flock_fd = -1;
-		    }
+		    funlockhistfile();
 #endif
 		}
 	    }
@@ -3158,10 +3164,7 @@ lockhistfile(char *fn, int keep_trying)
 
     if (ct == lockhistct) {
 #ifdef HAVE_FCNTL_H
-	if (flock_fd >= 0) {
-	    close(flock_fd);
-	    flock_fd = -1;
-	}
+	funlockhistfile();
 #endif
 	DPUTS(ret == 0, "BUG: return value non-zero on locking error");
 	return ret;
@@ -3191,10 +3194,7 @@ unlockhistfile(char *fn)
 	unlink(lockfile);
 	free(lockfile);
 #ifdef HAVE_FCNTL_H
-	if (flock_fd >= 0) {
-	    close(flock_fd);
-	    flock_fd = -1;
-	}
+	funlockhistfile();
 #endif
     }
 }
-- 
2.20.1


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

* [PATCH 3/3] Delay closing the history file until the fcntl-lock is released.
  2019-03-09  0:54       ` [PATCH 1/3] Move readhistfile() after flockhistfile() Philippe Troin
  2019-03-09  0:54         ` [PATCH 2/3] Factorize the code that unlock the fcntl() lock into funlockhistfile() Philippe Troin
@ 2019-03-09  0:54         ` Philippe Troin
       [not found]         ` <CAH+w=7aUD11M_GYy-FOC5MPGpGXb+o9O_q855OTC32fnSnpshQ@mail.gmail.com>
  2 siblings, 0 replies; 11+ messages in thread
From: Philippe Troin @ 2019-03-09  0:54 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Philippe Troin

Closes are not delayed if zsh is not compiled with fcntl() lock or if
the histfcntllock option isn't set.
---
 Src/hist.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 93 insertions(+), 4 deletions(-)

diff --git a/Src/hist.c b/Src/hist.c
index 981316674..2d0b3eb0f 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2527,6 +2527,63 @@ readhistline(int start, char **bufp, int *bufsiz, FILE *in)
 
 #ifdef HAVE_FCNTL_H
 static int flock_fd = -1;
+#define FLOCK_MAX_FILES_TO_CLOSE 10
+static int flock_fds_to_close[FLOCK_MAX_FILES_TO_CLOSE] = { -1 };
+static FILE* flock_files_to_close[FLOCK_MAX_FILES_TO_CLOSE];
+
+static int
+closehistfd(int fd) {
+    int i;
+
+    if (flock_fd < 0) {
+	return close(fd);
+    }
+
+    for (i=0; i < FLOCK_MAX_FILES_TO_CLOSE; ++i) {
+	if (flock_fds_to_close[i] < 0) {
+	    break;
+	}
+    }
+
+    if (i >= FLOCK_MAX_FILES_TO_CLOSE) {
+	zerr("Ran out of flock_fds_to_close, increase the "
+	     "FLOCK_MAX_FILES_TO_CLOSE limit (currently at %d) in hist.c.",
+	     FLOCK_MAX_FILES_TO_CLOSE);
+	/* Close if we can't postpone */
+	return close(fd);
+    }
+
+    flock_fds_to_close[i] = fd;
+
+    return 0;
+}
+
+static int
+closehistfile(FILE* fp) {
+    int i;
+
+    if (flock_fd < 0) {
+	return fclose(fp);
+    }
+
+    for (i=0; i < FLOCK_MAX_FILES_TO_CLOSE; ++i) {
+	if (! flock_files_to_close[i]) {
+	    break;
+	}
+    }
+
+    if (i >= FLOCK_MAX_FILES_TO_CLOSE) {
+	zerr("Ran out of flock_files_to_close, increase the "
+	     "FLOCK_MAX_FILES_TO_CLOSE limit (currently at %d) in hist.c.",
+	     FLOCK_MAX_FILES_TO_CLOSE);
+	/* Close if we can't postpone */
+	return fclose(fp);
+    }
+
+    flock_files_to_close[i] = fp;
+
+    return 0;
+}
 
 /*
  * Lock file using fcntl().  Return 0 on success, 1 on failure of
@@ -2539,10 +2596,18 @@ flockhistfile(char *fn, int keep_trying)
     struct flock lck;
     long sleep_us = 0x10000; /* about 67 ms */
     time_t end_time;
+    int i;
 
     if (flock_fd >= 0)
 	return 0; /* already locked */
 
+    /* Clear out the list of fds/file descriptors which will be closed
+       upon relinquising the lock */
+    memset(flock_files_to_close, 0, sizeof(flock_files_to_close[0])*FLOCK_MAX_FILES_TO_CLOSE);
+    for (i=0; i < FLOCK_MAX_FILES_TO_CLOSE; ++i) {
+	flock_fds_to_close[i] = -1;
+    }
+
     if ((flock_fd = open(unmeta(fn), O_RDWR | O_NOCTTY)) < 0)
 	return errno == ENOENT ? 0 : 2; /* "successfully" locked missing file */
 
@@ -2575,12 +2640,36 @@ flockhistfile(char *fn, int keep_trying)
 static void
 funlockhistfile()
 {
+    int i;
+
     if (flock_fd >= 0) {
 	close(flock_fd);
 	flock_fd = -1;
     }
+
+    for (i=0; i < FLOCK_MAX_FILES_TO_CLOSE; ++i) {
+	if (flock_files_to_close[i]) {
+	    fclose(flock_files_to_close[i]);
+	    flock_files_to_close[i] = NULL;
+	} else {
+	    break;
+	}
+    }
+
+    for (i=0; i < FLOCK_MAX_FILES_TO_CLOSE; ++i) {
+	if (flock_fds_to_close[i] >= 0) {
+	    close(flock_fds_to_close[i]);
+	    flock_fds_to_close[i] = -1;
+	} else {
+	    break;
+	}
+    }
 }
-#endif
+
+#else /* ! HAVE_FCNTL_H */
+#  define closehistfd(x)   close(x)
+#  define closehistfile(x) fclose(x)
+#endif /* ! HAVE_FCNTL_H */
 
 /**/
 void
@@ -2768,7 +2857,7 @@ readhistfile(char *fn, int err, int readflags)
 	zfree(buf, bufsiz);
 
 	popheap();
-	fclose(in);
+	closehistfile(in);
     } else if (err)
 	zerr("can't read history file %s", fn);
 
@@ -2860,7 +2949,7 @@ savehistfile(char *fn, int err, int writeflags)
 		if (fd >=0) {
 		    out = fdopen(fd, "w");
 		    if (!out)
-			close(fd);
+			closehistfd(fd);
 		} else
 		    out = NULL;
 	    }
@@ -2948,7 +3037,7 @@ savehistfile(char *fn, int err, int writeflags)
 		lasthist.text = ztrdup(start);
 	    }
 	}
-	if (fclose(out) < 0 && ret >= 0)
+	if (closehistfile(out) < 0 && ret >= 0)
 	    ret = -1;
 	if (ret >= 0) {
 	    if (tmpfile) {
-- 
2.20.1


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

* Re: [PATCH 1/3] Move readhistfile() after flockhistfile().
       [not found]               ` <3228a3e68f2580fc25a9fda9bf7ccf5ce9a73689.camel@fifi.org>
@ 2019-03-10  1:19                 ` Bart Schaefer
  2019-03-11 21:04                   ` Jason L Tibbitts III
  0 siblings, 1 reply; 11+ messages in thread
From: Bart Schaefer @ 2019-03-10  1:19 UTC (permalink / raw)
  To: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On Sat, Mar 9, 2019, 12:29 PM Philippe Troin <phil@fifi.org> wrote:

>
> By the way, on a completely unrelated note, on recent version of Fedora
> (running 29 here), I need to configure with CFLAGS=-I/usr/include/tirpc
> or the build fails with:
>
>    hashnameddir.c:49:12: fatal error: rpc/types.h: No such file or
> directory
>     #  include <rpc/types.h>
>                ^~~~~~~~~~~~~
>
> Do we need a feature check for libtirpc?
>

I don't know Fedora very well; feedback from the list would be beneficial.

But it sounds to me as though we do.

>

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

* Re: [PATCH 1/3] Move readhistfile() after flockhistfile().
  2019-03-10  1:19                 ` [PATCH 1/3] Move readhistfile() after flockhistfile() Bart Schaefer
@ 2019-03-11 21:04                   ` Jason L Tibbitts III
  2019-03-12  7:55                     ` Jun T
  0 siblings, 1 reply; 11+ messages in thread
From: Jason L Tibbitts III @ 2019-03-11 21:04 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

>>>>> "BS" == Bart Schaefer <schaefer@brasslantern.com> writes:

BS> I don't know Fedora very well; feedback from the list would be
BS> beneficial.

Note that Fedora in general doesn't have this problem.  I can certainly
build the zsh source RPM without issues on my Fedora systems, and
Fedora's buildsystem and package CI system also doesn't have any issues
with it.

I can't really guess as to what might be wrong with the system where
he's trying to build zsh.  In Fedora all packages are built in something
called mock which sets up a chroot with a limited set of things
installed within it.  This frees us from worrying too much about what
might be installed (or modified) on the host where the packages are
being built.  I do know (because
https://bugzilla.redhat.com/show_bug.cgi?id=1687574 was filed for this)
that the build isn't being done that way and so is susceptible to random
things being installed on the host system.

 - J<

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

* Re: [PATCH 1/3] Move readhistfile() after flockhistfile().
  2019-03-11 21:04                   ` Jason L Tibbitts III
@ 2019-03-12  7:55                     ` Jun T
  2019-03-12  9:52                       ` Peter Stephenson
  0 siblings, 1 reply; 11+ messages in thread
From: Jun T @ 2019-03-12  7:55 UTC (permalink / raw)
  To: zsh-workers


> 2019/03/12 6:04, Jason L Tibbitts III <tibbs@math.uh.edu> wrote:
> 
>>>>>> "BS" == Bart Schaefer <schaefer@brasslantern.com> writes:
> 
> BS> I don't know Fedora very well; feedback from the list would be
> BS> beneficial.
> 
> Note that Fedora in general doesn't have this problem.  I can certainly
> build the zsh source RPM without issues on my Fedora systems,

The problem occurs only if HAVE_NIS is defined, and it is defined only
if you are using NIS. In configure.ac, line 2083:

AH_TEMPLATE([HAVE_NIS],
[Define to 1 if you have NIS.])
AC_CACHE_CHECK(for NIS, zsh_cv_sys_nis,
[test -f /usr/bin/ypcat && /usr/bin/ypcat passwd.byname > /dev/null 2>&1 && \
zsh_cv_sys_nis=yes || zsh_cv_sys_nis=no])
if test x$zsh_cv_sys_nis = xyes; then
  AC_DEFINE(HAVE_NIS)
fi

so HAVE_NIS is defined only if "ypcat passwd.byname" succeeds, i.e.,
only if you are currently using NIS on your Fedora box.

RPC is removed from glibc-2.26; on Fedora 28 or later (and probably on
other recent/future distributions which use glib-2.26 or later) RPC
is in libtirpc whose headers are in /usr/include/tirpc/.

Below is a simple patch, but it is "minimal" and feel free to extend it.

# Is it better to allow anyone who has required libraries to build
# NIS-aware zsh even if he/she is not using NIS? Or add an option
# like --enable-nis?


diff --git a/configure.ac b/configure.ac
index 5513e25f1..8a2664ed2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2087,6 +2087,10 @@ AC_CACHE_CHECK(for NIS, zsh_cv_sys_nis,
 zsh_cv_sys_nis=yes || zsh_cv_sys_nis=no])
 if test x$zsh_cv_sys_nis = xyes; then
   AC_DEFINE(HAVE_NIS)
+dnl RPC is removed from glibc-2.26 and replaced by libtirpc
+  AC_CHECK_HEADER(rpc/rpc.h, [],
+  [test -f /usr/include/tirpc/rpc/rpc.h && \
+   CPPFLAGS="$CPPFLAGS -I/usr/include/tirpc"])
 dnl Some systems (Solaris 2.x, Linux Redhat 5.x) require
 dnl libnsl (Network Services Library) to find yp_all
   AC_SEARCH_LIBS(yp_all, nsl)



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

* Re: [PATCH 1/3] Move readhistfile() after flockhistfile().
  2019-03-12  7:55                     ` Jun T
@ 2019-03-12  9:52                       ` Peter Stephenson
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Stephenson @ 2019-03-12  9:52 UTC (permalink / raw)
  To: zsh-workers

On Tue, 2019-03-12 at 16:55 +0900, Jun T wrote:
> Below is a simple patch, but it is "minimal" and feel free to extend it.

I'm not Fedora-based any more, but my gut feel is minimal is probably
the right approach here.

Thanks
pws


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

end of thread, other threads:[~2019-03-12  9:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-27 18:30 Issues with fcntl() history file locking Philippe Troin
2019-02-27 21:27 ` Bart Schaefer
2019-02-28  6:36   ` Philippe Troin
2019-03-09  0:53     ` Philippe Troin
2019-03-09  0:54       ` [PATCH 1/3] Move readhistfile() after flockhistfile() Philippe Troin
2019-03-09  0:54         ` [PATCH 2/3] Factorize the code that unlock the fcntl() lock into funlockhistfile() Philippe Troin
2019-03-09  0:54         ` [PATCH 3/3] Delay closing the history file until the fcntl-lock is released Philippe Troin
     [not found]         ` <CAH+w=7aUD11M_GYy-FOC5MPGpGXb+o9O_q855OTC32fnSnpshQ@mail.gmail.com>
     [not found]           ` <82f4a6db638fbfce396e64a45029424185863068.camel@fifi.org>
     [not found]             ` <CAH+w=7ZS=ke8xHuBaO+hu0-RTtW=GYnG-0MENfBtTsyyp9joyg@mail.gmail.com>
     [not found]               ` <3228a3e68f2580fc25a9fda9bf7ccf5ce9a73689.camel@fifi.org>
2019-03-10  1:19                 ` [PATCH 1/3] Move readhistfile() after flockhistfile() Bart Schaefer
2019-03-11 21:04                   ` Jason L Tibbitts III
2019-03-12  7:55                     ` Jun T
2019-03-12  9:52                       ` Peter Stephenson

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).