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

 
 

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

Submitted by:  Dave <barx>
Submitted on:  Sun 19 Jan 2020 04:39:28 AM UTC  
 
Category:  Core Severity:  1 - Wish
Item Group:  New feature Status:  Need Info
Privacy:  Public Assigned to:  G. Branden Robinson <gbranden>
Open/Closed:  Open Planned Release:  None

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

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>
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>
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>
Project AdministratorIn charge of this item.
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>
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>
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>
Project 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>
Project AdministratorIn charge of this item.
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>

 

(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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    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.5