GNU bug report logs - #61927
[PATCH 0/1] environment: Fix '--check' for shells that output ANSI escape codes based on 'TERM'.

Previous Next

Package: guix-patches;

Reported by: Winter <winter <at> winter.cafe>

Date: Fri, 3 Mar 2023 02:26:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 61927 in the body.
You can then email your comments to 61927 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#61927; Package guix-patches. (Fri, 03 Mar 2023 02:26:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Winter <winter <at> winter.cafe>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Fri, 03 Mar 2023 02:26:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Winter <winter <at> winter.cafe>
To: guix-patches <at> gnu.org
Cc: Winter <winter <at> winter.cafe>
Subject: [PATCH 0/1] environment: Fix '--check' for shells that output ANSI
 escape codes based on 'TERM'.
Date: Thu,  2 Mar 2023 21:25:35 -0500
Hi,

I ran into an issue where 'guix shell --check' would fail on Debain 11's Bash, even though the environment variable it was complaining about not being set was in fact set.

As far as I can tell, the issue is caused by Bash's support for bracketed paste [0], which is causing it to output "\x1b[?2004l\r" before the script output, leading the first vhash entry to be invalid (in the sense that 'validate-child-shell-environment' will never see it).

Per ncurses' terminfo database [1], setting it to nothing (which is "unknown," per my understanding/testing with 'tput') is the same as setting it to "dumb," which will hopefully get us in the clear for the future, assuming shells play nice with regards to terminal specifications and detection.

Given this, there are a few alternative solutions that I'd like to propose:

1. Set 'TERM' to 'dumb'. This has the benefit of being explictly specified in the terminfo database, though any sane implementation should(?) fall back to it if it's set to nothing instead.
2. Use the shell non-interactively, if possible, and fall back to the script in interactive mode. We'd have to assume that the shell takes some sort of argument for a command (e.g. '-c'/'--command'), though based on the fact that we currently don't do that, I assume it's for a good reason.
3. Strip ANSI escape codes when adding the lines to the list. This might be the best one, though I'm unsure how best to do it. A PEG parser? A regex?

This was definitely an interesting issue to look into, and I look forward to thoughts on the matter.

Thanks,
Winter

P.S. I've seen a few commit prefixes that touch 'guix/scripts', like "environment" (as I've used here, since it seems to be the most prominent) and "guix: shell". Is one more correct than the other?

[0]: https://en.wikipedia.org/wiki/ANSI_escape_code#CSI_(Control_Sequence_Introducer)_sequences, under "Some popular private sequences"
[1]: https://invisible-island.net/ncurses/terminfo.src.html#tic-unknown

Winter (1):
  environment: Unset 'TERM' when checking environment.

 guix/scripts/environment.scm | 2 ++
 1 file changed, 2 insertions(+)


base-commit: ea870a70e93e7ea93e35fa8e0afc3cfdb1b66ba2
-- 
2.39.1





Information forwarded to guix-patches <at> gnu.org:
bug#61927; Package guix-patches. (Fri, 03 Mar 2023 02:28:02 GMT) Full text and rfc822 format available.

Message #8 received at 61927 <at> debbugs.gnu.org (full text, mbox):

From: Winter <winter <at> winter.cafe>
To: 61927 <at> debbugs.gnu.org
Cc: Winter <winter <at> winter.cafe>
Subject: [PATCH 1/1] environment: Unset 'TERM' when checking environment.
Date: Thu,  2 Mar 2023 21:27:10 -0500
* guix/scripts/environment.scm (child-shell-environment): Unset 'TERM'.
---
 guix/scripts/environment.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 44cfcb4f76..a3cc9e6b83 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -514,6 +514,8 @@ (define lines
        (catch #t
          (lambda ()
            (load-profile profile manifest #:pure? #t)
+           ; We don't want ANSI escape codes such as bracketed paste to mess up the output of the script.
+           (setenv "TERM" "")
            (setenv "GUIX_ENVIRONMENT" profile)
            (close-fdes controller)
            (login-tty inferior)
-- 
2.39.1





Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Mon, 13 Mar 2023 13:22:01 GMT) Full text and rfc822 format available.

Notification sent to Winter <winter <at> winter.cafe>:
bug acknowledged by developer. (Mon, 13 Mar 2023 13:22:02 GMT) Full text and rfc822 format available.

Message #13 received at 61927-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Winter <winter <at> winter.cafe>
Cc: 61927-done <at> debbugs.gnu.org
Subject: Re: bug#61927: [PATCH 0/1] environment: Fix '--check' for shells
 that output ANSI escape codes based on 'TERM'.
Date: Mon, 13 Mar 2023 14:21:44 +0100
Hi Winter,

Winter <winter <at> winter.cafe> skribis:

> I ran into an issue where 'guix shell --check' would fail on Debain 11's Bash, even though the environment variable it was complaining about not being set was in fact set.

OK.

> As far as I can tell, the issue is caused by Bash's support for bracketed paste [0], which is causing it to output "\x1b[?2004l\r" before the script output, leading the first vhash entry to be invalid (in the sense that 'validate-child-shell-environment' will never see it).

I didn’t know about bracketed paste; my understanding is that it’s the
terminal that emits those sequences, and Bash is merely supposed to
interpret them no?  (E.g., Bash should make sure pasted text is not
immediately executed.)

Regardless, there’s the possibility of such escape sequences popping up.

> Per ncurses' terminfo database [1], setting it to nothing (which is "unknown," per my understanding/testing with 'tput') is the same as setting it to "dumb," which will hopefully get us in the clear for the future, assuming shells play nice with regards to terminal specifications and detection.
>
> Given this, there are a few alternative solutions that I'd like to propose:
>
> 1. Set 'TERM' to 'dumb'. This has the benefit of being explictly specified in the terminfo database, though any sane implementation should(?) fall back to it if it's set to nothing instead.
> 2. Use the shell non-interactively, if possible, and fall back to the script in interactive mode. We'd have to assume that the shell takes some sort of argument for a command (e.g. '-c'/'--command'), though based on the fact that we currently don't do that, I assume it's for a good reason.
> 3. Strip ANSI escape codes when adding the lines to the list. This might be the best one, though I'm unsure how best to do it. A PEG parser? A regex?

I’m fine with setting ‘TERM’ to the empty string or to “dumb”.

Option #2 wouldn’t let us test what we want (we really want to test an
interactive shell); option #3 sounds tricky and fragile to me.

So I tweaked the comment and commit log and applied your patch.

Thank you for the investigation and fix!

Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 11 Apr 2023 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 352 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.