Hi Chris, Did you observe this behaviour inside a git repo directory? I wonder if this git security thing could be relevant: https://lwn.net/Articles/892755/ It makes also me wonder about readline completion stuff possibly interacting. Isn't that implemented with readline?
I have had some mystery bash parsing errors, and I noticed set|less shows a heck of a lot of functions defined that I don't remember seeing in the past. Anyway, shouldn't stuff like that have better hygiene than just prefixed _underscore ? Or maybe set|less doesn't show all that on your system? Disclaimer: I played a lot of games trying to make stuff conditional at login, where I renamed .bash_profile and .bashrc (e.g. .my_bashrc) which brought .profile into play, and I messed with the downstream of that to source some .my_'s conditionally, so I've go a fragile mess right now ;/ Anyway, did you determine why things changed in the first place? Or will this be a whack-a-mole game with future weirdnesses? :) Semms like IWBN to have interfaces governed by contracts :) Best, Bengt Richter On +2022-06-19 13:40:50 -0700, Chris Marusich wrote: > Hi Ludo, > > Thank you for the review! > > Ludovic Courtès <l...@gnu.org> writes: > > > LGTM, please push! > > Before pushing, I did some more tests to make sure it was still working. > When I did this, I noticed that read-line was no longer returning > strings that end in "\r". This prevents child-shell-environment from > behaving correctly, since it incorrectly assumes that all the lines end > in "\r", stripping it off unconditionally. In the past, I'm sure > read-line was returning strings that end in "\r". I don't know what > changed, but I've attached a second patch that fixes this issue, also. > > Unless you have more feedback, I'll go ahead and push both patches to > master in a few days. > > -- > Chris > > PGP: https://savannah.gnu.org/people/viewgpg.php?user_id=106836 > From c4fee9e63f8cb694de86ae46bd1e2e4c692eb6f6 Mon Sep 17 00:00:00 2001 > From: Chris Marusich <cmmarus...@gmail.com> > Date: Sun, 19 Jun 2022 13:16:04 -0700 > Subject: [PATCH] environment: Don't assume that lines have a trailing "\r". > > I've noticed that the child-shell-environment procedure is misbehaving on my > computer because the lines returned by read-line do not have a trailing "\r". > In the past, I recall that such lines did in fact have a trailing "\r". I'm > not sure why it changed, but it seems prudent to just rewrite this code to > tolerate both cases, since it seems that both cases can happen. > > * guix/scripts/environment.scm (child-shell-environment) [lines]: Instead of > checking if the line exactly matches "GUIX_CHECK_DONE\r"; check if the line > begins with "GUIX_CHECK_DONE". Instead of always stripping the trailing > character from the line, only do it if the line has a trailing "\r". > --- > guix/scripts/environment.scm | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm > index f0cb341aab..1fb4f5b7c6 100644 > --- a/guix/scripts/environment.scm > +++ b/guix/scripts/environment.scm > @@ -462,13 +462,18 @@ (define lines > ;; prompt from getting mixed into what we read. > (match (read-line shell-pipe-in) > ((? eof-object?) (reverse lines)) > - ("GUIX-CHECK-DONE\r" > + ((? (lambda (line) > + ;; The line might or might not have a > trailing \r. > + (string-prefix? "GUIX-CHECK-DONE" line))) > (display "done\n" port) > (reverse lines)) > (line > - ;; Drop the '\r' from LINE. > - (loop (cons (string-drop-right line 1) > - lines)))))))) > + ;; Strip the trailing '\r' from LINE if present. > + (let ((stripped-line > + (if (string-suffix? "\r" line) > + (string-drop-right line 1) > + line))) > + (loop (cons stripped-line lines))))))))) > (close-port port) > (close-port shell-pipe-in) > (waitpid pid) > -- > 2.34.0 >