Bug 218614 - sysutils/e2fsprogs: Patch test to reduce reliance on GNU extensions
Summary: sysutils/e2fsprogs: Patch test to reduce reliance on GNU extensions
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Matthias Andree
URL:
Keywords:
Depends on:
Blocks: 218385
  Show dependency treegraph
 
Reported: 2017-04-13 00:23 UTC by Kyle Evans
Modified: 2017-04-21 23:45 UTC (History)
1 user (show)

See Also:
mandree: maintainer-feedback+


Attachments
svn(1) diff of sysutils/e2fsprogs (1.37 KB, patch)
2017-04-13 00:23 UTC, Kyle Evans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle Evans freebsd_committer freebsd_triage 2017-04-13 00:23:24 UTC
Created attachment 181743 [details]
svn(1) diff of sysutils/e2fsprogs

Hi,

I'm attaching a patch to an e2fsprogs test to use egrep instead of grep to reduce reliance on GNU extensions; in this case, branching in a BRE is a GNU extension. This was identified in a recent exp-run to test installing BSD grep as /usr/bin/grep and disable GNU grep compatibility in the process.

I did not patch the output since it's effectively the same expression. If you think this could be worth trying to upstream, I'll happily redo the patch to also patch the output and help in any way I can.
Comment 1 Matthias Andree freebsd_committer freebsd_triage 2017-04-21 21:58:07 UTC
I'm a bit loathe to add a "deprecated" tool such as egrep (which is grep -E), let alone propose introducing a deprecated tool name to the upstream maintainer, but portability is generally welcomed by him.

Would a simple stupid sed 's/egrep/grep -E/g' on your patch still work for BSD grep?

Do we have a Wiki page on the GNU -> BSD grep migration so I know how to replace "grep" by BSD for proper testing?
Comment 2 Kyle Evans freebsd_committer freebsd_triage 2017-04-21 22:07:26 UTC
Yup, that will indeed work fine.

I do have some information on the wiki on the "BSDgrep" page, but it seems to be lacking the "use WITH_BSD_GREP in src.conf to test and recompile usr.bin/grep" bit.
Comment 3 Matthias Andree freebsd_committer freebsd_triage 2017-04-21 22:39:02 UTC
I can't seem to provoke the error from http://pb2.nyi.freebsd.org/data/headamd64PR218385-default/2017-04-09_09h40m49s/logs/errors/e2fsprogs-1.43.4.log on 10.3-RELEASE-p18 amd64. Without or with the patch, the self-test suite passes.

Here's what I did:
- update /usr/src from https://svn.freebsd.org/base/releng/10.3 
- then:
  499  cd /usr/src/
  500  svn up
  505  cd usr.bin/grep/ 
  512  sudo make WITH_BSD_GREP=yes clean
  513  sudo make WITH_BSD_GREP=yes cleandir
  514  sudo make WITH_BSD_GREP=yes cleandir
  515  sudo make WITH_BSD_GREP=yes clean
  516  sudo make WITH_BSD_GREP=yes clean
  518  sudo make WITH_BSD_GREP=yes obj
  519  sudo make WITH_BSD_GREP=yes depend
  520  sudo make WITH_BSD_GREP=yes all
  521  sudo make WITH_BSD_GREP=yes install

- /usr/bin/grep --version
grep (BSD grep) 2.5.1-FreeBSD

- which -a grep
/usr/bin/grep

- rebuild e2fsprogs from scratch on the bare metal (so I don't introduce any non-updates jails using GNU grep)

What else do I need to do to prove that the patch is (a) required, (b) effective?
Comment 4 Matthias Andree freebsd_committer freebsd_triage 2017-04-21 22:41:10 UTC
recompiling grep with the same sequence as given below, but omitting the WITH_BSD_GREP=yes  also gives me a grep that has "BSD grep" in the output, exact the same way.

Does WITH_BSD_GREP work on 10.3?
Comment 5 Matthias Andree freebsd_committer freebsd_triage 2017-04-21 22:48:55 UTC
OK, the trick to building a non-GNUism-enabled BSD grep appears to be "make WITH_BSD_GREP=42  WITHOUT_GNU_COMPAT=yes " - but again that's indistinguishable in "grep --version", which is a pity.
Comment 6 commit-hook freebsd_committer freebsd_triage 2017-04-21 22:59:18 UTC
A commit references this bug:

Author: mandree
Date: Fri Apr 21 22:59:12 UTC 2017
New revision: 439131
URL: https://svnweb.freebsd.org/changeset/ports/439131

Log:
  Remove GNUism in grep basic regular expression in self-test.

  Versus Kyle's submission, replace "egrep" by "grep -E" to avoid
  reintroducing deprecated tools.

  PR:		218614
  Submitted by:	Kyle Evans <bsdports@kyle-evans.net>

Changes:
  head/sysutils/e2fsprogs/files/patch-tests_r__inline__xattr_script
Comment 7 Matthias Andree freebsd_committer freebsd_triage 2017-04-21 22:59:33 UTC
I'll point the upstream maintainer to our new patch so he can pull it upstream.
Comment 8 Matthias Andree freebsd_committer freebsd_triage 2017-04-21 23:06:32 UTC
1.2 h total time
Comment 10 Kyle Evans freebsd_committer freebsd_triage 2017-04-21 23:45:16 UTC
(In reply to Matthias Andree from comment #9)

Whoops, sorry about that. Thanks for the addition! As of r317255, GNU_GREP_COMPAT is actually disabled by default, with GNU extensions to be re-introduced when we can work out another option for a libregex.

I've put up for review the following to add "GNU Compatible" to the version string when compiled with GNU extensions: https://reviews.freebsd.org/D10451

We should be close enough on flag/option parity to make such a claim if the GNU extensions are included.