bugGNU roff - Bugs: bug #57630, test-groff clobbers environment's...

 
 

bug #57630: test-groff clobbers environment's GROFF_*_PATH variables

Submitter:  Dave <barx>
Submitted:  Sun 19 Jan 2020 04:39:28 AM UTC
   
 
Category:  Core Severity:  1 - Wish
Item Group:  Build/Installation Status:  Rejected
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 08 Oct 2020 12:16:37 PM UTC, comment #10: 

After four months without discussion, I'm closing this one for now.  I don't think test-groff is intended for use like a real system executable; mainly it drives the regression tests, as Ingo noted.

But that's not all I use it for.

I also have some shell aliases 'tg' and 'tgu' (the latter adding the -Tutf8 parameter) which I run all the time from my groff git checkout.  I can't even count how many experiments I've conducted this way, using a build tree's groff as a kind of REPL.


$ alias tg tgu
alias tg='./build/test-groff'
alias tgu='./build/test-groff -Tutf8'


I recommend that close observers of (and participants in!) groff development do something similar.

I'm a bit way of changing test-groff such that it operates in a fused installation/build environment, something is never seen in any other circumstance.

If someone wants to use such a thing as a means of uncovering possible bugs, then I advise copying the script to their $HOME/bin or similar and making the changes documented earlier in this bug's discussion.  And then, if problems turn up, trying to construct a minimal reproducer script to get at the misbehavior.

I'm therefore resolving this bug as wontfix, without prejudice, to use a U.S. legal metaphor.

G. Branden Robinson <gbranden>
Group administrator
Wed 27 May 2020 07:34:51 PM UTC, comment #9: 

I'm not sure that brought us any closer to consensus.  Only one person admitted to using the script (though immediately after my query, Ingo posted a diatribe against its use, which could have skewed the results), and he did not express an opinion on this issue, only saying that he found the script useful.

My (obviously biased) opinion is that those who use the script ought to have more of a say in how it functions than those who don't.  While I sympathize with Ingo's concerns about ongoing maintenance complexity, I also think the script as it stands is fairly simple, that its git log shows very few noncosmetic changes over the years, and that the proposed change, if implemented as described in comment #1, adds minimal complexity.

But as its heaviest use is probably in regression testing, which to date Branden has been almost entirely responsible for, I'm willing to defer to his judgment on this, and on the larger existential question he posed in comment #7.

Dave <barx>
Group Member
Fri 14 Feb 2020 02:47:56 AM UTC, comment #8: 

Feedback solicited from the email list: http://lists.gnu.org/archive/html/groff/2020-02/msg00019.html

Dave <barx>
Group Member
Thu 13 Feb 2020 10:45:22 AM UTC, comment #7: 

It seems like we don't really have a consensus on this one yet.

Maybe we need an actual definition of what it is 'test-groff' is actually supposed to do.

Ersatz user-local-packaging of the built groff tree as a runtime?

Ersatz isolated containerization of the built groff tree as a runtime?

These objectives are in tension.

G. Branden Robinson <gbranden>
Group administrator
Fri 24 Jan 2020 09:57:22 PM UTC, comment #6: 


comment #5:

> comment #4:
> > I did this in my version of "test-groff", but without any check as I
> > knew the variable had a value.
>
> Ah, I see now that bug #55924 contains a patch that covers GROFF_TMAC_PATH (but not the other two *PATH variables).  Is that what you're referring to here?


No exactly, as I had forgotten that I had submitted that report when I wrote the above.

Bjarni Ingi Gislason <bjarniig>
Mon 20 Jan 2020 11:05:03 AM UTC, comment #5: 

comment #4:

> I did this in my version of "test-groff", but without any check as I
> knew the variable had a value.


Ah, I see now that bug #55924 contains a patch that covers GROFF_TMAC_PATH (but not the other two *PATH variables).  Is that what you're referring to here?

Dave <barx>
Group Member
Mon 20 Jan 2020 01:21:55 AM UTC, comment #4: 


original submission:

> The test-groff script simulates an installation by defining its own GROFF_BIN_PATH, GROFF_FONT_PATH, and GROFF_TMAC_PATH values pointing to the build directory.
>
> Unfortunately, instead of appending to any existing values of these variables, it overwrites them, so the user's environment is ignored.
>
> For this script to more closely emulate running an installed groff, it should prepend any existing values to the ones it defines.  This would make it treat these variables the way a groff installation would, searching the user's environment (if any) first, then the "system" one.
>
> For example:
>


> if [ "$GROFF_FONT_PATH" ]
> then
>   GROFF_FONT_PATH="$GROFF_FONT_PATH$SEP"
> fi
>
> GROFF_FONT_PATH=$GROFF_FONT_PATH\
> [... rest of script's normal definition for this variable ...]


  I did this in my version of "test-groff", but without any check as I
knew the variable had a value.

  After reading this, I realised that my change was not that what I
wanted.

  The environment of "test-groff" should be contained to the git
repository with a possibility to add new sources to test (experiment
with).

  If something is wrong with that environment, I can compare it with my
standard environment by using the installed "nroff" or "groff".

  So I changed "GROFF_TMAC_PATH" to "TEST_GROFF_TMAC_PATH" to make it
independent of any standard environmental variable.

  One can obviously set

TEST_GROFF_TMAC_PATH=${GROFF_TMAC_PATH}

in ones environment,
if one wants to test (experiment) with that.

Bjarni Ingi Gislason <bjarniig>
Sun 19 Jan 2020 08:56:10 PM UTC, comment #3: 

comment #2:

> The test-groff crutch is not intended as a user command.
> If it has any usefulness at all (and even that may be dubious),


Any user who finds it useless shouldn't care much how it works. And those who find it useful are in a better position to make the call on how it's most useful.

Still, most of those people probably aren't following this bug report, so this might be a discussion for the email list.

> it is for running regression tests.


Certainly one ought to be able to include various settings of these *PATH variables in a regression test.  So even for the limited use case you're willing to cede it, making it behave more like a real groff installation is important.

Dave <barx>
Group Member
Sun 19 Jan 2020 12:51:06 PM UTC, comment #2: 

I consider this a feature, not a bug.

The test-groff crutch is not intended as a user command.
If it has any usefulness at all (and even that may be dubious), it is for running regression tests.

So the more predictable and the less dependent on external influcences it is, the better.

It should not be optimized for versatility, but strictly for minimalism.

I suggest to close this ticket as invalid.

Ingo Schwarze <schwarze>
Group Member
Sun 19 Jan 2020 06:50:54 AM UTC, comment #1: 

Just as a note, there is an even more idiomatic was to do this with POSIX parameter expansion:


GROFF_FONT_PATH=${GROFF_FONT_PATH:+$GROFF_FONT_PATH$SEP}WHATEVER


G. Branden Robinson <gbranden>
Group administrator
Sun 19 Jan 2020 04:39:28 AM UTC, original submission:  

The test-groff script simulates an installation by defining its own GROFF_BIN_PATH, GROFF_FONT_PATH, and GROFF_TMAC_PATH values pointing to the build directory.

Unfortunately, instead of appending to any existing values of these variables, it overwrites them, so the user's environment is ignored.

For this script to more closely emulate running an installed groff, it should prepend any existing values to the ones it defines.  This would make it treat these variables the way a groff installation would, searching the user's environment (if any) first, then the "system" one.

For example:


if [ "$GROFF_FONT_PATH" ]
then
  GROFF_FONT_PATH="$GROFF_FONT_PATH$SEP"
fi

GROFF_FONT_PATH=$GROFF_FONT_PATH\
[... rest of script's normal definition for this variable ...]


Dave <barx>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bjarniig (Posted a comment)
  • -email is unavailable- added by schwarze (Posted a comment)
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by barx (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-03-05 gbranden Item GroupFeature change Build/Installation
    2020-10-08 gbranden StatusNeed Info Rejected
        Open/ClosedOpen Closed
    2020-02-13 gbranden StatusNone Need Info
    2020-01-19 schwarze Severity3 - Normal 1 - Wish
        Assigned toNone gbranden

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code