bugGNU roff - Bugs: bug #59442, [PATCH] [groff] move soelim before...

 
 

bug #59442: [PATCH] [groff] move soelim before preconv in constructed pipeline

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Tue 10 Nov 2020 05:26:03 PM UTC
   
 
Category:  General Severity:  1 - Wish
Item Group:  Feature change Status:  Need Info
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Open Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 15 Jul 2024 01:45:07 AM UTC, comment #15: 

comment #13:
...

> $ cat try.roff
> I'll eat
> .so the\ one.roff
> soon.
...


> What seems nuts about that is that the first error seems like a pointless restriction that, if lifted, would make the second error disappear.


Fixing that sounds like a job for bug #65108.

G. Branden Robinson <gbranden>
Group administrator
Mon 15 Jul 2024 01:34:55 AM UTC, comment #14: 

(I was a little surprised, when crafting that example, that I had to say "groff -Tascii" rather than "nroff" because nroff lacks the -s option.  All this means is I forgot about bug #64450.)

Dave <barx>
Group Member
Mon 15 Jul 2024 12:42:53 AM UTC, comment #13: 

comment #12:

> we're already sunk, and have been for decades, by the simple
> obstacle of a space character.


Frustratingly, soelim can deal with this and raw troff can't.  From soelim(1): "To embed a space [in the filename], write "\ " (backslash followed by a space)."

But this works only with soelim.  I've scaled down the example to remove the potentially confounding factor of the non-ASCII characters (and to reflect cutbacks on grocery expenditures due to inflation).

$ cat try.roff
I'll eat
.so the\ one.roff
soon.
$ cat 'the one.roff'
my sandwich
$ groff -Tascii try.roff | cat -s
troff:try.roff:2: error: an escaped ' ' is not allowed in an identifier
troff:try.roff:2: error: can't open 'the': No such file or directory
I'll eat soon.

$ groff -s -Tascii try.roff | cat -s
I'll eat my sandwich soon.

What seems nuts about that is that the first error seems like a pointless restriction that, if lifted, would make the second error disappear.

> GNU troff could have read file name arguments more in the
> fashion that it did string definitions and appendments, by
> reading until the newline.


That would work as well, but it would be nice if groff and soelim handled filename spaces the same way.

> I am consequently inclined to either reject this ticket or
> postpone it (again) until after groff 1.24.0.


Postponing is fine; the issue today is no worse than it's been for the past however-many years, and there seems little complaint about it.  That could just as easily argue for rejecting it, but I feel like "I want to preconv my .so'd files" is a perfectly reasonable ask.  And if soelim comes first but the user doesn't want the .so'd files run through preconv, it's easy to say "preconv | groff -s".  There's not a good analogue for vice versa.

Dave <barx>
Group Member
Thu 11 Jul 2024 01:27:21 PM UTC, comment #12: 

I've remained stuck on this because of the questions Dave raises in comment #11 and the groff simulator in my brain not being as good as experimentation.

So I did an experiment.

To my surprise, the following worked.


$ locale charmap
UTF-8
$ cat EXPERIMENTS/entrée.troff
the croque monsieur
$ cat EXPERIMENTS/so-a-fancy-filename.troff
.sp
We'll begin the meal with
.so entrée.troff
then continue with the paella
and wash it down with a stein of mead.
$ groff -I EXPERIMENTS -a EXPERIMENTS/so-a-fancy-filename.troff
<beginning of page>
We'll begin the meal with the croque monsieur then continue with the paella and wash it down with a stein of mead.


Impressive, but fragile.  (Be aware that `-I` forces soelim to run.)

The only reason this works is because the encoding of the text file "so-a-fancy-filename.troff" is the same as the encoding used by the file system to store the file name "entrée.troff".

There will be situations where that is not true.  And if we run preconv on such a file, it seems more likely to become less true, simply because it multiplies the possibilities for character encoding conversion.

Things fall apart with a rather more modest file name character--the space.  If I rename "entrée.troff" to "the entrée.troff" and update the `so` request in "so-a-fancy-filename.troff" accordingly, here's what happens.


$ groff -I EXPERIMENTS -a EXPERIMENTS/so-a-fancy-filename.troff
soelim:./EXPERIMENTS/so-a-fancy-filename.troff:3: error: can't open 'the': No such file or directory
<beginning of page>
troff:./EXPERIMENTS/so-a-fancy-filename.troff:3: error: can't open 'the': No such file or directory
We'll begin the meal with then continue with the paella and wash it down with a stein of mead.


Let's take soelim out of the equation and see what happens.


$ cat EXPERIMENTS/so-a-fancy-filename.troff
.sp
We'll begin the meal with
.so EXPERIMENTS/the entrée.troff
then continue with the paella
and wash it down with a stein of mead.
$ groff -a EXPERIMENTS/so-a-fancy-filename.troff
<beginning of page>
troff:EXPERIMENTS/so-a-fancy-filename.troff:3: error: can't open 'EXPERIMENTS/the': No such file or directory
We'll begin the meal with then continue with the paella and wash it down with a stein of mead.


So leaving aside fancy non-basic Latin characters, we're already sunk, and have been for decades, by the simple obstacle of a space character.

Worse, this limitation was completely unnecessary: no troff request that takes a file name as a request argument locates it anywhere but as the last argument, which means GNU troff could have read file name arguments more in the fashion that it did string definitions and appendments, by reading until the newline.

(I don't propose to support newlines embedded in file names, and would oppose doing so, even if POSIX 2024, finally, finally made it easier to cope with these.  Solaris 10 will remain lobotomized in a forgotten corner of the asylum^Wdata center, drooling pitiably.


$ find . -print0
find: bad option -print0
find: [-H | -L] path-list predicate-list
bash-3.2$ xargs -0
xargs: illegal option -- 0
xargs: Usage: xargs: [-t] [-p] [-e[eofstr]] [-E eofstr] [-I replstr] [-i[replstr]] [-L #] [-l[#]] [-n # [-x]] [-s size] [cmd [args ...]]
bash-3.2$ read -d
bash: read: -d: option requires an argument
read: usage: read [-ers] [-u fd] [-t timeout] [-p prompt] [-a array] [-n nchars] [-d delim] [name ...]


)


I am consequently inclined to either reject this ticket or postpone it (again) until after groff 1.24.0.

Feedback?

G. Branden Robinson <gbranden>
Group administrator
Mon 06 May 2024 05:43:34 AM UTC, comment #11: 

It sounds like a decision is needed on which is the cart and which the horse.  Should this bug take priority -- as comment #1 points out, "The whole point of soelim is to get preprocessors to run on `.so`ed files" -- and then bug #65108 has to figure out how to work within that framework?  Or should filename handling take priority, and then this bug needs to figure out how to implement the comment #1 precept within that new framework?

Dave <barx>
Group Member
Tue 23 Apr 2024 01:06:48 PM UTC, comment #10: 

Sorry, I did read comment #5 before posting but missed the implication of its effect on the soelim proposal.

Dave <barx>
Group Member
Tue 23 Apr 2024 06:15:42 AM UTC, comment #9: 

Oh, what I said in comment #8 isn't news at all, given comment #5 and comment #6.

Might help if I scrolled down and read sometimes before replying...

G. Branden Robinson <gbranden>
Group administrator
Tue 23 Apr 2024 06:13:38 AM UTC, comment #8: 

Thanks, Dave.

It did occur to me at one point that this change might interfere with one of the deeper linguistic reforms I'm considering to the formatter, like bug #65108.

But I've forgotten my reasoning and didn't write it down.

G. Branden Robinson <gbranden>
Group administrator
Tue 23 Apr 2024 05:42:20 AM UTC, comment #7: 

This was made "Need Info" when Branden asked "Can anyone else think of any objections?"  As none have arisen in over three years, I'm resetting the status.

With no theoretical problems on the table, perhaps the best way to shake out any actual problems is to go ahead and make the change, giving any unforeseen issues ample opportunity to raise their ugly heads in testing from developers now, and from a wider audience later when groff goes into rc mode.

Dave <barx>
Group Member
Wed 03 Jan 2024 02:25:27 AM UTC, comment #6: 

comment #5:

> It occurs to me that I hadn't actually written up this idea with these
> particulars before.  This should probably be a new ticket.


Now bug #65108.

Dave <barx>
Group Member
Tue 02 Jan 2024 10:03:34 PM UTC, comment #5: 

comment #4:

> comment #1:
> > 1. What happens if a .so(urced) file has a non-ASCII character
> > in its filename?
>
> This seems more likely to work with the proposed change than without
> it.  Currently, with preconv preceding soelim, preconv will change the
> line ".so naïve_file" into ".so na\[u00EF]ve_file", guaranteed to
> fail.  If soelim comes first, it at least has a chance of succeeding.


Another thing I want to do is specialize the formatter's logic when
handling file name arguments given to requests.

Presently, GNU troff calls the same internal function to gather an
argument that is a file name as it does to gather a *roff identifier.

Maybe that made sense in 1990, but it doesn't today.  File names can
contain spaces and non-ASCII characters (in whatever encoding the file
system happens to support).

Since these arguments used mainly as-is, handed off to standard C
library functions like `fopen()`, I don't anticipate many problems here
(O Fortuna, seize my hostage).  The only exception to that I can think
of off the top of my head is the value of the `.F` register, which
interpolates a file name.  We will need some way to represent this such
things as output.  At first blush, it seems to me that we can
interpolate spaces as-is (if you want the argument quoted, do that
yourself in context), and any unprintable non-Basic Latin bytes in
groff's \[u00xx] notation.

I say "\[u00xx]" instead of "\[uXXXX]" because we have no way of knowing
what the file system's character encoding is.  Might be ISO 8859-1,
UTF-8, UTF-16BE/LE, or something else entirely.

What would be affected by this:

Requests:

cf
fp (when invoked with a 3rd argument)
hpf
hpfa
lf (when invoked with a 2nd argument)
mso
msoquiet
nx (when invoked with a 2nd argument)
open
opena
psbb
so
soquiet
trf

Escape sequences:

\O5 (but since this is mainly used internally to manage temporary files
    by grohtml, maybe lazily postponing this in hope that my Grand Plan
    to revise grohtml to no longer use a dedicated preprocessor is a
    better idea)


Registers:

.F

It occurs to me that I hadn't actually written up this idea with these
particulars before.  This should probably be a new ticket.

And probably step 1 would be a simple refactor to introduce file name
argument-gathering and -interpolating functions which initially behave
no differently than the status quote, but simply wrap existing logic for
identifier gathering and whatever one-off thing the `.F` interpolator
does.

Regards,
Branden

G. Branden Robinson <gbranden>
Group administrator
Tue 02 Jan 2024 08:02:32 PM UTC, comment #4: 

comment #1:

> 1. What happens if a .so(urced) file has a non-ASCII character
> in its filename?


This seems more likely to work with the proposed change than without it.  Currently, with preconv preceding soelim, preconv will change the line ".so naïve_file" into ".so na\[u00EF]ve_file", guaranteed to fail.  If soelim comes first, it at least has a chance of succeeding.

Dave <barx>
Group Member
Tue 29 Aug 2023 03:46:43 AM UTC, comment #3: 

comment #2:

> this may indeed be worth considering, but after release, not before.


That time has arrived!

The patch as posted still applies fine (even despite the "verbatim" tags being malformed, which can sometimes cause problems), but the original submission makes clear that the patch is incomplete as-as, pointing out:

> the documentation must then mention this fact in the
> explanation of the "-s" option.

Dave <barx>
Group Member
Wed 11 Nov 2020 11:38:28 AM UTC, comment #2: 

Quick answer without doing a full analysis: this may indeed be worth considering, but after release, not before.  This is not a bugfix but a fundamental change in behaviour, and as such, it could in principle cause serious regressions in existing documents.

Moving to Item Group "Preprocessor preconv" und Category "New feature" because in a nutshell, the idea here is to apply preconv to additional content.

Ingo Schwarze <schwarze>
Group Member
Wed 11 Nov 2020 07:37:27 AM UTC, comment #1: 

Bjari's point seems sound to me.  The whole point of soelim is to get preprocessors to run on `.so`ed files (`.mso`, `.pso`).

I'm trying to think of cases where this would break things where they aren't already broken (we have no test cases here, of course...yet).

Questions to ponder, regarding both current behavior and what happens if we accept the proposed patch.

1. What happens if a .so(urced) file has a non-ASCII character in its filename?  soelim(1) does not seem to speak to this question. Maybe because it is assuming preconv(1) has already been run, or, more likely I think, the issue was not considered when the page was written, since it long predates preconv itself.

2. What about EBCDIC hosts?  Even plain ASCII code points are complete disarranged in EBCDIC.  soelim uses standard C library functions for character comparison and handling (see src/preproc/soelim/soelim.cpp:do_file()).  There are no EBCDIC worries today because preconv has already busted the input down to 7-bit chars.  What about after this patch?  We can't assume that's been done, so chars with the 8th bit set might be in the input.  But I'm not seeing any problems; anything that soelim doesn't care about as documented in its man page, it passes through without alteration, amounting to `putchar(getc());`.  And the chars that soelim DOES care about should not be spuriously matched in a UTF-8 sequence, because all continuation bytes in a UTF-8 sequence have the high bit set.  soelim will, therefore, not spurious match them and munge them.

Can anyone else think of any objections?

G. Branden Robinson <gbranden>
Group administrator
Tue 10 Nov 2020 05:26:03 PM UTC, original submission:  

src/roff/groff/groff.cpp: correct the order of preprocessors in the
pipeline.

  The "soelim" command must be the first in the pipeline to incorporate
 all sourced files into the input file,
 before it is sent to the next command.

  Change the order from "preconv, soelim, refer" to
"soelim, preconv, refer".

  As "preconv" can't (currently) figure out the encoding,
 if it gets its input from standard input,
 "groff" must be called with the option "-K <encoding>" or
 "-D <encoding>", and not just with "-k",
 if the encoding is not the default one.

  This means that the documentation must then mention this fact in the
explanation of the "-s" option.
.verbatim+
Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
 src/roff/groff/groff.cpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/roff/groff/groff.cpp b/src/roff/groff/groff.cpp
index f568ac02..cd61dbd0 100644
--- a/src/roff/groff/groff.cpp
+++ b/src/roff/groff/groff.cpp
@@ -53,9 +53,11 @@ extern "C" {
 
 // grap, chem, and ideal must come before pic;
 // tbl must come before eqn
-const int PRECONV_INDEX = 0;
-const int SOELIM_INDEX = PRECONV_INDEX + 1;
-const int REFER_INDEX = SOELIM_INDEX + 1;
+// soelim must come first to complete the input file for the later
+// commands in the pipeline
+const int SOELIM_INDEX = 0;
+const int PRECONV_INDEX = SOELIM_INDEX +1;
+const int REFER_INDEX = PRECONV_INDEX + 1;
 const int GRAP_INDEX = REFER_INDEX + 1;
 const int CHEM_INDEX = GRAP_INDEX + 1;
 const int IDEAL_INDEX = CHEM_INDEX + 1;
--
2.28.0

-verbatim-

Bjarni Ingi Gislason <bjarniig>

 

(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

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by barx (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 bjarniig (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 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-07-11 gbranden StatusNone Need Info
        Summary[PATCH] groff.cpp: move soelim before preconv in constructed pipeline [PATCH] [groff] move soelim before preconv in constructed pipeline
    2024-04-23 barx StatusNeed Info None
    2024-01-02 gbranden Dependencies- bugs #65099 is dependent
    2024-01-02 gbranden Summary[PATCH] groff.cpp: move soelim before preconv in constructed pipeline [PATCH] groff.cpp: move soelim before preconv in constructed pipeline
    2022-06-10 gbranden CategoryPreprocessor preconv General
        Severity3 - Normal 1 - Wish
    2020-11-11 schwarze CategoryCore Preprocessor preconv
        Item GroupIncorrect behaviour Feature change
    2020-11-11 gbranden Summary[PATCH] groff.cpp: correct the order of preprocessors in the pipeline [PATCH] groff.cpp: move soelim before preconv in constructed pipeline
    2020-11-11 gbranden Item GroupFeature change Incorrect behaviour
        StatusNone Need Info
        Assigned toNone gbranden
    2020-11-10 barx Carbon-CopyRemoved 93119 -
    2020-11-10 barx Summarygroff.cpp: correct the order of preprocessors in the pipeline [PATCH] groff.cpp: correct the order of preprocessors in the pipeline

    Back to the top

    Powered by Savane 3.13-b921.
    Corresponding source code