bugGNU roff - Bugs: bug #44768, [PATCH] gdiffmk(1) is not Solaris...

 
 

bug #44768: [PATCH] gdiffmk(1) is not Solaris 10 /bin/sh compatible

Submitter:  None
Submitted:  Wed 08 Apr 2015 07:05:21 AM UTC
   
 
Category:  Utilities Severity:  3 - Normal
Item Group:  Crash/Unresponsive Status:  Fixed
Privacy:  Public Assigned to:  bgarrigues
Open/Closed:  Closed Planned Release:  1.22.4
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 02 May 2022 01:26:02 AM UTC, comment #27: 


commit 64db09953a9f725077c4697bc290906f9770a80d
Author: Bertrand Garrigues <bertrand.garrigues@laposte.net>
Date:   Mon May 4 00:01:35 2015 +0200

    Fix Savannah bug #44768 (Solaris 10 compatibility issues).

    * m4/groff.m4 (GROFF_DIFF_D): new macro to check if `diff' has
    option -D, and search for `gdiff' if not (new variable DIFF_PROG
    will contain the `diff' program to be used).

    * m4/groff.m4 (GROFF_HAVE_TEST_EF_OPTION): new macro to check if
    `test' supports the option -ef (created new variable
    HAVE_TEST_EF_OPTION).

    * m4/groff.m4 (GROFF_BASH): new macro to search for `bash' (new
    variable BASH_PROG will contain the full path to `bash' or
    `/bin/sh' if `bash' was not found).

    * configure.ac: use the 3 new macros defined in m4/groff.m4.

    * contrib/gdiffmk/gdiffmk.am: set HAVE_TEST_EF_OPTION, BASH_PROG,
    DIFF_PROG in the generated script `gdiffmk'.

    * contrib/gdiffmk/gdiffmk.sh:
    - Fix incorrect space after '!' in sed script regex.
    - `test -ef' is not portable: If `bash' is not available and the
    shell does not support `test -ef', the option to pass a third
    argument as the output file is disabled.
    - Report an error if `diff' does not support -D option.
    - GNU diff and Solaris diff support -D, but #ifndef / #endif are
    treated differently (GNU adds a '!' in the #endif): modify the
    regexs in the sed script accordingly.


G. Branden Robinson <gbranden>
Group administrator
Sun 03 May 2015 10:06:47 PM UTC, comment #26: 

Happy to hear that it worked!  I commited the patch.  Following Peter's suggestion I also added from the 1st patch (in groff.m4) the test of the support of -D option in diff.

Regards,

Bertrand Garrigues

Bertrand Garrigues <bgarrigues>
Group administrator
Sun 03 May 2015 10:56:29 AM UTC, comment #25: 

Bertrand ...

If Peter is happy, and  make check  is happy, I'm happy.
Many and many thanks to both of you and Werner for all your efforts here.

Mike

Michael H Bianchi <mbianchi>
Group Member
Sun 03 May 2015 12:48:54 AM UTC, comment #24: 

Bertrand & Co,

Tested on Solaris 10 Update 10 via `gmake dist' tarball - No Issues :-)

Autoconf selected bash, Solaris diff, Solaris sed. As bash is standard in Solaris 10 (as previously stated), the code should work on a standard Solaris 10 system.

I personally think the first patch's diff -D code could probably be added back to the final commit, as its a big world, and not all diff(1) implementations may have the -D option. Also the work has been done and verified (well by me at least), seems a waste to toss it.

Many Thanks to Bertrand for the patches!!!

Regards,
Peter

Peter Bray <illumino>
Sat 02 May 2015 03:23:52 PM UTC, comment #23: 

Here is a new patch.

1. Diff

As Peter said, both GNU diff and Solaris diff support -D, but #ifndef / #endif are treated differently (GNU adds a '!' in the #endif). The easiest way to solve this problem seems to modify the regexp (currently either a '!' or a 'not' was mandatory).

2. test -ef

OK ls -i is not the solution, stat is not posix, so I proceeded as follows:
  - Test if the shell supports test -ef
  - Test if bash is available, and use it by defautl in gdiffmk (otherwise we keep /bin/sh).
In gdiffmk, if both tests were not successful, the option to pass a third file as the output is disabled (gdiffmk returns an error, make check will fail).

3. Sed

I don't think that ! are the problem, but the space after the exclamation mark is clearly not correct.  I forgot to remove one space.

Peter, Mike, could you please test?

Regards,

Bertrand Garrigues


(file #33881)

Bertrand Garrigues <bgarrigues>
Group administrator
Mon 27 Apr 2015 05:01:01 AM UTC, comment #22: 

Of course, groff already has perl(1) dependencies, ...

--
UK Family Comedy: Outnumbered
Children's Grandmother pointing to a grey paper mask with a trunk, in the corner of the room, "What's that?". Children's Father, "We don't speak about that".  Grandmother: "Why Not?". Father: "It's the ..."

Peter Bray <illumino>
Mon 27 Apr 2015 04:21:51 AM UTC, comment #21: 

Another post to attach the last two files.

Also Note: `ls -i' has the possibility of falsely denying the user his/her request if the file arguments are on different file systems but have the same inode number.

This is trivial on ZFS to demonstrate:

# mkfile 128M /tmp/mypool
# zpool create mypool /tmp/mypool
# zfs create mypool/fs1
# zfs create mypool/fs2
# touch /mypool/fs1/file
# touch /mypool/fs2/file
# ls -i /mypool/fs1/file
         8 /mypool/fs1/file
# ls -i /mypool/fs2/file
         8 /mypool/fs2/file
# zpool destroy mypool
# rm /tmp/mypool


(file #33797, file #33798)

Peter Bray <illumino>
Mon 27 Apr 2015 04:12:23 AM UTC, comment #20: 

Bertrand, Werner & Mike,

Patch applied and tested on Solaris 10 Update 10 (X86_64).

Notes

  1. configure choose Solaris' diff implementation as it has -D

  2. bug_44768_diff_sh_sed.patch.test-suite.log is the test-suite.log
     from applying the patch and testing from a "gmake dist" tarball

  3. bug_44768_diff_sh_sed.patch.with-gdiff.test-suite.log is the
     test-suite.log, of the patch with the ./gdiffmk updated to use
     "gdiff" as its diff command. No other changes.

  4. GNU diff and Solaris diff's -D options diff in the closing comment
     with GNU diff placing a ! in front of the condition name.
     See Attached: diff_-D_XXXXXXs-side-by-side and the raw files
     diff_-D_XXXXXX_file1_file2.diff_solaris and
     gdiff_-D_XXXXXX_file1_file2.diff_gnu

  5. Back in followup-notes-2 (comment 16) I started writing that I thought
     was the ! as Bertrand assumed, and like Bertrand I looked at the doco,
     in my case the "sed and awk Pocket Reference" and Solaris 10 sed(1)
     manual page, but as stated ! was implemented. See sed.manual-page-selected

  6. I started this point with I think is the regexp it is breaking on, BUT
     then I tested further, concluded that the regexp is not the problem.

% echo '#else /* XXXX'
#else /* XXXX

% echo '#else /* XXXX'  |  sed -e '/^#else \/\* XXXX/ p'
#else /* XXXX
#else /* XXXX

% echo '#else /* XXXX'  | gsed -e '/^#else \/\* XXXX/ p'
#else /* XXXX
#else /* XXXX

  7. I think it is the second part of the line, as in the following:

% echo '#else /* XXXX'  | sed -e '/^#else \/\* XXXX/ { p }'
sed: command garbled: /^#else \/\* XXXX/ { p }

% echo '#else /* XXXX'  | gsed -e '/^#else \/\* XXXX/ { p }'
#else /* XXXX
#else /* XXXX

      but I don't have the time today to move this further along.
      I'll submit this as is and try and find show time in the next
      day or so.

Regards,
Peter


(file #33793, file #33794, file #33795, file #33796)

Peter Bray <illumino>
Sun 26 Apr 2015 04:34:17 PM UTC, comment #19: 

Hi Mark, Peter,

I tried to solve the 3 problems mentioned previously (attached a patch bug_44768_diff_sh_sed.patch).

1. Diff

-D option is mandatory, so we have to use GNU diff. In groff.m4 I define a DIFF_PROG command (set to diff by default).  I first check if diff is working.  If not, I'm looking for gdiff (I assume that people would install GNU diff as gdiff on a standard path). If gdiff is not available, I let the default command as diff: the error message in gdiffmk will explain the problem.

2. test -ef

Instead to force the use of bash, I propose to use ls -i, which gives the inode of the file.

3. Sed

I was about to implement the same solution than for diff. However the error on Solaris:
"Unrecognized command: /^#else \/\* _diffmk_18037_/ ! {"
looked stranged: the command has nothing special (no GNU extension) and should work. But POSIX says: "A function can be preceded by one or more '!' characters, in which case the function shall be applied if the addresses do not select the pattern space. Zero or more <blank> characters shall be accepted before the first '!' character. It is unspecified whether <blank> characters can follow a '!' character, and conforming applications shall not follow a '!' character with <blank> characters."
So it seems that the space after the '!' character is not correct (I removed it in the patch).

Then, I made more tests by forcing the option --posix to the sed command: this causes make check to fail on my Arch Linux environment. The sed script of gdiffmk uses a GNU extension that allows to match a regexp or another regexp: \(not\|!\), this in order to match lines like
#else /* not _diffmk_xxxx_
or
#else /* ! _diffmx_xxxx_

So I split this it in two sets of pattern tests and commands.

Could you please test on Solaris 10 ?

Regards,

Bertrand Garrigues


(file #33791)

Bertrand Garrigues <bgarrigues>
Group administrator
Sat 25 Apr 2015 07:27:41 AM UTC, comment #18: 

Thanks.  I'm reassigning this to Bertrand :-)

Hopefully, he finds some time to take care of @BASH@ and friends – however, I think it would be better to make gdiffmk work with standard sh, too...

Werner LEMBERG <wl>
Group administrator
Sat 25 Apr 2015 01:34:23 AM UTC, comment #17: 

PS: Only tested Solaris bash 3.2.51 on an older Solaris 10 Update 10 system

Peter Bray <illumino>
Sat 25 Apr 2015 01:32:40 AM UTC, comment #16: 

Mike (& Werner),

As you suggested the script still does not run on Solaris 10
natively. See attached test-suite.log-Solaris10-sh-sed-diff.

Reasons:
  - Solaris 10 /bin/sh does not support test's -ef test
  - GNU sed is required,
  - GNU diff is required for reasons mentioned in usage

Changing the script to #! /bin/bash fixes the script complaint about
test's -ef option. /bin/bash is standard on Solaris 10 (version
3.00.16 on Sun Solaris 10 Update 8 and version 3.2.51 on Oracle
Solaris 10 update 11) .

I have GNU sed compiled locally as gsed, and GNU diff as gdiff,
neither of these are normally available on Solaris 10.

If I make these changes to the build script (./gdiffmk) than re-run
"gmake check", I get a clean result

Changing gsed back to sed results in the attached
test-suite.log-Solaris10-sed-GNU-diff-bash, which shows that it is
more than the failure to parse one expression that Solaris sed(1) can
not handle. As per my previous suggestion, "Developed with GNU sed(1),
other implementations not verified"

The only remaining changes that could be applied are the previously
mentioned @BASH@, @DIFF@, @SED@ substitutions at build time. But I
think Mike can consider the script complete from his point of view.

Regards,
Peter


(file #33777, file #33778)

Peter Bray <illumino>
Fri 24 Apr 2015 01:04:32 PM UTC, comment #15: 

((My first git use.
Thanks to the authors of
    http://savannah.gnu.org/maintenance/UsingGit/
    groff/TESTS
))

Include baseline.6a  baseline.9a

Michael H Bianchi <mbianchi>
Group Member
Fri 24 Apr 2015 02:59:49 AM UTC, comment #14: 

Mike,

Note to self: read the output of all commands.

I just went through a clean build cycle only to get the same results, turns out the 'gmake dist' did not work because the file `contrib/gdiffmk/tests/baseline.6a' could not be found. Thus I used the old tarball and ...

As to the question marks in the usage. I'll humbly suggest:
 "Developed with GNU sed(1), other implementations not verified"

Regards,
Peter

Peter Bray <illumino>
Thu 23 Apr 2015 06:08:22 PM UTC, comment #13: 

I think I successfully committed the changes, but I doubt the
result is completely Solaris 10 compatible.

ChangeLog entry ...
2015-04-13  Mike Bianchi <MBianchi@Foveal.com>
        More fixes to Savannah bug #44768.

        * gdiffmk.sh:
                replace
                        for OPTION   with  while [ $# -gt 0 ]

                        test -e ...  with  test -f

                        use  ${DIFFCMD}  for the last  diff
                add
                        -s SEDCMD  option
                        OPTION="$1"

                many other cosmetic changes documented in the bug Discussion.
                especially:
                        capitalize variables  e.g.  CMD=`basename $0`
 
        fix  make check  tests within gdiffmk
        * tests/gdiffmk_tests.sh
                add  set -e
                        fails if ${abs_top_builddir} not set or incorrect

        * tests/runtests.sh
                add test 6a
                        # Different values for addmark, changemark, deletemark
                        # Alternate format of -a -c and -d flag arguments

                add test 9a
                        # Test -D  and  -M  options
                        # Alternate format of -M argument.

                add printout of failure count
                add exit with failure exit_code if any test fails

Michael H Bianchi <mbianchi>
Group Member
Tue 14 Apr 2015 02:13:37 AM UTC, comment #12: 

Michael,

I think the best and most portable approach would be to have the autoconf infrastructure find and substitute in appropriate default values for DIFFCMD and SEDCMD via DIFFCMD=@DIFF@ and SEDCMD=@SED@, I know that is how grep(1) and egrep(1) is handled by the groff project currently.

My guess is that we would need to add something to configure.ac, and maybe a makefile to make this happen.

The reason is that, not all platforms (including Solaris 10) have GNU implementations of these commands (the Solaris 10 case), or implementations called "diff" and "sed" (my case, where I compile and install them with a "g" program prefix), so modifying the PATH won't help.

The #! line could be treated similarly, with an @BASH@, or just as you suggest #!/bin/bash, and as you say that fixes so many other issues!

Re: Authorship I'll say no :-)

Regards,
Peter

PS: I watched the UNIX wars, from a far, I was not aware of the PWB Shell (other PWB tools, yes, indirectly) and the earlier Shell Wars. Many thanks, I have some additional reading to do. But its not even close to being there, lucky you! And I agree, our education continues, its one of the great things about computing, but especially UNIX, you never stop learning!

Peter Bray <illumino>
Mon 13 Apr 2015 02:31:43 PM UTC, comment #11: 

It occurs to me that iff the GNU compatible commnads live in a predictable place on Solaris (and elsewhere),
say  /usr/gnu/bin/  , then we need only put that directory early in the path to find the compatible  diff, sed and test commands.

Is that the case?

Michael H Bianchi <mbianchi>
Group Member
Mon 13 Apr 2015 02:01:28 PM UTC, comment #10: 


> Michael is the Author.


I am happy to share authorship.  I am 67 years old and won't be here forever.
       What say you, Peter?  Want to share with me?
Everything I see says to me you should.

> 0. The canonical reference for sh compatibility issues the automake manual.


I witnessed the Shell Wars at Bell Labs in the 70s.  John Mashey and Steve Bourne had a friendly rivalery while Brian Kernighan was referee.  Both shells improved as a result, and Bourne's was the "winner" in the end.

 #! /bin/sh  is a permanent error because you cannot say simply what it means.
Until there is a /bin/posix_sh from Gnu, this is going to remain a mess.  M4sh illustrates that point, I claim.

In my opinion,  /bin/bash  should be portable, a standard GNU offering, and therefore a reasonable requirement for shell scripts.
 #! /bin/bash  would fix all these problems.


> - Solaris sed(1) is not up to the task of showing deletions but GNU sed(1) is. Added -s SEDCMD
> - DIFFCMD was set but not used in the most critical place ;-)
> - if ! <command> is not portable, replaced with "test $? -ne 0"

All Done.
((But shouldn't we able to require GNU diff and sed as prerequisites?))

> - test <file> -ef <file> is not portable (NO FIX PROVIDED)

I consider this Shell Wars.  Still broken.

> - inconsistent uppercase vs lowercase use in variable names (NOT FIXED) fixed only cmd -> CMD
> - $# and $? are numbers and don't need quoting (to the best of my knowledge)

A matter of style, but in this case I think you are right enough.  The script is easier to read this way.
All Done.

> - I always use ${1} rather than $1, that way ${10} does not look out of place, and all variables references are the same (${thing}).

A matter of style.  Not changed.


> - in theory any file path could contain spaces (thankfully most Unix people

never do that), but most user provided variables should be surrounded by
double quotes (at least that is what I have found in my 25+ years working
with many different Bourne shell implementations)
Done.

> - replaced "for VAR" usage with "while test $# -gt 0"

That was so wrong!  I hang my head is shame.
Done.

> - numeric comparisons in Bourne shell are -lt -le -eq -ne -ge -gt, though many people seem to use "${x}" = "3", and this is probably reasonable, if you don't if x actually contains a number, the numeric operator break badly for the empty string and non-numeric values.

Numeric comparisons are unquoted.

The attached script that seems to work for the basic case (two file
arguments to stdout), the three file arguments does not work due to
the use of "test <f> -ef <f>".
I did not take your fixes from this file.  Instead I did them by hand because I already had been stomping through the file.

Further testing is still required. <shell> -n <file> passes for
Solaris 10 /bin/sh, Solaris 10 ksh, zsh 4.3.9 and bash 4.0.0 (this is
an older system). But as demonstrated this does not detect things like
"test <f> -ef <f>".

I've extended the tests in  contrib/gdiffmk/tests to include both the  -aA and -a A cases, and fixed  runtests.sh .
 make check  now passes or fails properly.  Before is always Passed, even when things were completely broken.

Commit coming soon.  Thanks for the pointers Werner.  My education continues ...

Michael H Bianchi <mbianchi>
Group Member
Mon 13 Apr 2015 06:29:58 AM UTC, comment #9: 


> 0. ego-less programming is nice, but a piece of software needs an author.
> Whom shall I contact in case of difficulties?


I agree to a point, I still think Michael is the Author.
The author of fixes (minor or portability), are a lot like "copy editors" or "book editors", sure we make the product better, but we are not the one doing the hard initial work of authoring. Michael and my writing styles are quite different ;-) Its Michael's work, I'm just trying to help others enjoy it.

> 0. The FSF is not (yet) able to properly handle patches that are put into the
> public domain – does this exist at all in the Australian jurisdiction?  We
> simply have to wait until your papers are signed, sent back, and processed by
> the FSF, which certainly takes a few weeks.  Sorry for the inconvenience.


Unknown, IANAL. Weeks is an electronic age - so sad, the law just has not moved with the times, but than that is always the case.

Very interesting point about public domain. I was just of the assumption that if a modify a work, and give that work back to author and expressly state they do not have to acknowledge my efforts and may claim the work as their own, should they see fit, that would suffice, but the law is an ...! (you read that as "another matter entirely" I'm sure) ;-)

Lets hope they accepted scanned signed copies at least, and are not going to make be sign something longer than "War and Peace" ;-)

> 0. The canonical reference for sh compatibility issues the automake manual.
> See
>
> https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Portable-Shell.html#Portable-Shell


Yes a very good reference which I came across again only recently.

Regards,
Peter

Peter Bray <illumino>
Mon 13 Apr 2015 04:50:09 AM UTC, comment #8: 

Thanks!  Two remarks.

0. ego-less programming is nice, but a piece of software needs an author.  Whom shall I contact in case of difficulties?

0. The FSF is not (yet) able to properly handle patches that are put into the public domain – does this exist at all in the Australian jurisdiction?  We simply have to wait until your papers are signed, sent back, and processed by the FSF, which certainly takes a few weeks.  Sorry for the inconvenience.

0. The canonical reference for sh compatibility issues the automake manual.  See

https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Portable-Shell.html#Portable-Shell

Werner LEMBERG <wl>
Group administrator
Mon 13 Apr 2015 01:45:54 AM UTC, comment #7: 

Greetings,

Further testing and hacking shows the following
- Solaris sed(1) is not up to the task of showing deletions
  but GNU sed(1) is. Added -s SEDCMD
- DIFFCMD was set but not used in the most critical place ;-)
- if ! <command> is not portable, replaced with "test $? -ne 0"
- test <file> -ef <file> is not portable (NO FIX PROVIDED)
- inconsistent uppercase vs lowercase use in variable names (NOT FIXED)
  fixed only cmd -> CMD
- $# and $? are numbers and don't need quoting (to the best of my knowledge)
- I always use ${1} rather than $1, that way ${10} does not look out of place,
  and all variables references are the same (${thing}).
- in theory any file path could contain spaces (thankfully most Unix people
  never do that), but most user provided variables should be surrounded by
  double quotes (at least that is what I have found in my 25+ years working
  with many different Bourne shell implementations)
- replaced "for VAR" usage with "while test $# -gt 0"
- numeric comparisons in Bourne shell are -lt -le -eq -ne -ge -gt, though
  many people seem to use "${x}" = "3", and this is probably reasonable, if
  you don't if x actually contains a number, the numeric operator break
  badly for the empty string and non-numeric values.

The attached script that seems to work for the basic case (two file
arguments to stdout), the three file arguments does not work due to
the use of "test <f> -ef <f>".

Further testing is still required. <shell> -n <file> passes for
Solaris 10 /bin/sh, Solaris 10 ksh, zsh 4.3.9 and bash 4.0.0 (this is
an older system). But as demonstrated this does not detect things like
"test <f> -ef <f>".

I have attached the whole script (the original .sh file, not the
generated one), rather than a patch, it up to you what you deem
reasonable for inclusion in the groff code base. And I personally do
not wish to be credited as an author (I try to practice ego-less
programming).

Regards,
Peter

PS: Formalities: I have requested copyright assignment "paperwork" via
assign@gnu.org, but that probably won't arrive until tomorrow my
time. I hereby donate the changes to the author / current maintainer
to do with as he/she pleases, without the need or requirement for
attribution, copyright or ownership in any form, now or in the future.


(file #33662)

Peter Bray <illumino>
Sun 12 Apr 2015 03:57:17 PM UTC, comment #6: 

I'm stuck.

I'm trying to build groff so I can run the tests but
$ ./bootstrap
./bootstrap: Error: 'automake' version == 1.11.6 is too old
./bootstrap:        'automake' version >= 1.12.2 is required

./bootstrap: Please install the prerequisite programs

But I cannot find a way to install 1.12.2 on

$ lsb_release -d
Description:    Debian GNU/Linux 7.8 (wheezy)



Michael H Bianchi <mbianchi>
Group Member
Sun 12 Apr 2015 02:45:54 PM UTC, comment #5: 

Mike, you have write permission, so please proceed.

Werner LEMBERG <wl>
Group administrator
Sun 12 Apr 2015 11:20:28 AM UTC, comment #4: 

I have been trying to figure out what was going on with the diffmk arguments
and wondering how the  for OPTION  loop ever worked with all those shifts.
I was about to try exactly the fix you came up with this morning.

I suggest changing  $2  to  "$2"  on all the  RequiresArgument  calls.

Many thanks for tackling this.
Pleasse add  your name below the  Written  by line.

                                                                Mike

Michael H Bianchi <mbianchi>
Group Member
Sun 12 Apr 2015 02:24:00 AM UTC, comment #3: 

Sorry I was wrong about the "shift", the right number of shifts are present.

I decided to continue to test against Solaris 10 /bin/sh, and the following patch (attached to preserve whitespace) seems to be fine.

Comments:
  - Changed the for loop to a while loop
  - Not all test(1) commands support -e (which is truly sad)

Basic execution runs to completion. Modified Line shows change bar :-)

Regards,
Peter

--- ./contrib/gdiffmk/gdiffmk.sh.orig 2015-04-10 19:37:02.502452458 +1000
+++ ./contrib/gdiffmk/gdiffmk.sh 2015-04-12 11:58:07.794180659 +1000
@@ -82,7 +82,7 @@
  ;;
  esac
 
- if test ! -e "$2"
+ if test ! -f "$2"
  then
  Exit $1 "File \`$2' not found."
  fi
@@ -159,8 +159,10 @@
 DIFFCMD=diff
 D_option=
 br=.br
-for OPTION
+
+while [ $# -gt 0 ]
 do
+ OPTION="${1}"
  case "${OPTION}" in
  -a*)
  ADDMARK=`RequiresArgument "${OPTION}" $2` &&


(file #33643)

Peter Bray <illumino>
Fri 10 Apr 2015 11:17:28 AM UTC, comment #2: 

Greetings,

  sh -n .../gdiffmk test passed on Solaris 10 (X86_64) ;-)

  .../gdiffmk --version test passed

  .../gdiffmk --help test passed

  sh/ksh/bash/zsh .../gdiffmk -a A -c C -d D filename1 filename2

  All FAIL with "gdiffmk:  Incorrect number of arguments."

All shells show "+ test 6 -lt 2 -o 6 -gt 3"
when a invoked with a -x argument to the shell

I believe the "shift"s need to be changed to "shift 2", as the option value in the second argument, so two shifts are needed to all options taking an argument.

Regards,
Peter Bray
Sydney, Australia

Anonymous
Fri 10 Apr 2015 07:44:36 AM UTC, comment #1: 

I've applied your patch, thanks (with slight whitespace fixes).

Mike, please test also!

Werner LEMBERG <wl>
Group administrator
Wed 08 Apr 2015 07:05:21 AM UTC, original submission:  

Greetings,

When building the git version of groff (2015/04/04) to check the fix
for https://savannah.gnu.org/bugs/?44708, I discovered that the
installed gdiffmk(1) did not function, with the error:

syntax error at line 22: `cmd=$' unexpected

Of all the installed shell scripts (11 in total), gdiffmk(1) is the
only one to use the $(...) syntax, instead of the older `...` syntax,
and thus the only one to fail Solaris 10 a test of the form:
/bin/sh -n <file>

Only a seven instances exist:
grep -n '$(' /pkgs/64-bit/release/groff-git_2014_04_04/bin/gdiffmk

22:cmd=$( basename $0 )
112:                      "Cannot write directory \`$( dirname "$2" )'."
166:            ADDMARK=$( RequiresArgument "${OPTION}" $2 )            &&
170:            CHANGEMARK=$( RequiresArgument "${OPTION}" $2 )         &&
174:            DELETEMARK=$( RequiresArgument "${OPTION}" $2 )         &&
181:            MARK1=$( RequiresArgument "${OPTION}" $2 )              &&
194:            DIFFCMD=$( RequiresArgument "${OPTION}" $2 )            &&

The attached patch, allows the script to pass /bin/sh -n test on
Solaris 10, as well as simple run tests such as, 'gdiffmk' and
'gdiffmk --version', and 'gmake check' (from source root), no further
testing has not been conducted.

Regards,
Peter Bray
Sydney, Australia

PS: This patch is submitted in the hope that it will be useful, but
WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. I hereby donate
it to the groff project, to do with as the maintainer(s) see fit,
including distribution under any license or licenses they deem fit,
now or in the future. Copyright may be assigned as the maintainer(s)
see fit. It yours!

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #33881:  bug_44768_patch_2.patch added by bgarrigues (4KiB - text/x-patch)
file #33797:  diff_-D_XXXXXXs-side-by-side added by illumino (670B - application/octet-stream)
file #33798:  sed.manual-page-selected added by illumino (2KiB - application/octet-stream)
file #33793:  bug_44768_diff_sh_sed.patch.test-suite.log added by illumino (2KiB - application/octet-stream)
file #33794:  bug_44768_diff_sh_sed.patch.test-suite.log added by illumino (2KiB - application/octet-stream)
file #33795:  gdiff_-D_XXXXXX_file1_file2.diff_gnu added by illumino (304B - application/octet-stream)
file #33796:  diff_-D_XXXXXX_file1_file2.diff_solaris added by illumino (302B - application/octet-stream)
file #33777:  test-suite.log-Solaris10-sh-sed-diff added by illumino (3KiB - application/octet-stream)
file #33778:  test-suite.log-Solaris10-sed-GNU-diff-bash added by illumino (2KiB - application/octet-stream)
file #33662:  gdiffmk.sh added by illumino (7KiB - application/x-sh)
file #33643:  gdiffmk.patch added by illumino (449B - application/octet-stream)
file #33589:  groff-git_2014_04_04-solaris_bin_sh.patch added by None (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by bgarrigues (Updated the item)
  • -email is unavailable- added by mbianchi (Posted a comment)
  • -email is unavailable- added by illumino (Updated the item)
  • -email is unavailable- added by wl (Posted a comment)
  •  

    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 21 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-04-01 gbranden Planned ReleaseNone 1.22.4
    2022-05-02 gbranden CategoryNone Utilities
        Summarygdiffmk(1) is not Solaris 10 /bin/sh compatible - PATCH provided [PATCH] gdiffmk(1) is not Solaris 10 /bin/sh compatible
    2015-06-30 bgarrigues Open/ClosedOpen Closed
    2015-05-03 bgarrigues StatusReady for Merge Fixed
    2015-05-02 bgarrigues Attached File- Added bug_44768_patch_2.patch, #33881
    2015-04-27 illumino Attached File- Added diff_-D_XXXXXXs-side-by-side, #33797
        Attached File- Added sed.manual-page-selected, #33798
    2015-04-27 illumino Attached File- Added bug_44768_diff_sh_sed.patch.test-suite.log, #33793
        Attached File- Added bug_44768_diff_sh_sed.patch.test-suite.log, #33794
        Attached File- Added gdiff_-D_XXXXXX_file1_file2.diff_gnu, #33795
        Attached File- Added diff_-D_XXXXXX_file1_file2.diff_solaris, #33796
    2015-04-26 bgarrigues Attached File- Added bug_44768_diff_sh_sed.patch, #33791
    2015-04-25 wl Assigned tombianchi bgarrigues
    2015-04-25 illumino Attached File- Added test-suite.log-Solaris10-sh-sed-diff, #33777
        Attached File- Added test-suite.log-Solaris10-sed-GNU-diff-bash, #33778
    2015-04-13 illumino Attached File- Added gdiffmk.sh, #33662
    2015-04-12 illumino Attached File- Added gdiffmk.patch, #33643
    2015-04-10 wl StatusNone Ready for Merge
        Assigned toNone mbianchi
    2015-04-08 None Attached File- Added groff-git_2014_04_04-solaris_bin_sh.patch, #33589

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code