Patchwork gnu: Add re2.

login
register
mail settings
Submitter Marius Bakke
Date Aug. 17, 2016, 3:18 p.m.
Message ID <87bn0rzanb.fsf@ike.i-did-not-set--mail-host-address--so-tickle-me>
Download mbox | patch
Permalink /patch/14708/
State New
Headers show

Comments

Marius Bakke - Aug. 17, 2016, 3:18 p.m.
Marius Bakke <mbakke@fastmail.com> writes:

> Leo Famulari <leo@famulari.name> writes:
>
>> On Mon, Aug 15, 2016 at 02:05:16PM +0100, Marius Bakke wrote:
>>> 
>>> I wasn't sure where to put this, so went with its own file. It does not
>>> fully implement PCRE so pcre.scm seems inappropriate. Perhaps that could
>>> be renamed to regex.scm or similar.
>>
>> The patch looks good, but please put it in regex.scm! But, I don't want
>> to move all the regex packages into this new regex module. Perhaps tre,
>> since it appears to have no users in our tree.
>
> Moved to regex.scm. Thanks!

..and here is a patch that moves tre.scm over as well. The code is
unchanged apart from license: prefix.
Leo Famulari - Aug. 18, 2016, 8:43 p.m.
On Wed, Aug 17, 2016 at 04:18:48PM +0100, Marius Bakke wrote:
> Marius Bakke <mbakke@fastmail.com> writes:
> 
> > Leo Famulari <leo@famulari.name> writes:
> >
> >> On Mon, Aug 15, 2016 at 02:05:16PM +0100, Marius Bakke wrote:
> >>> 
> >>> I wasn't sure where to put this, so went with its own file. It does not
> >>> fully implement PCRE so pcre.scm seems inappropriate. Perhaps that could
> >>> be renamed to regex.scm or similar.
> >>
> >> The patch looks good, but please put it in regex.scm! But, I don't want
> >> to move all the regex packages into this new regex module. Perhaps tre,
> >> since it appears to have no users in our tree.
> >
> > Moved to regex.scm. Thanks!
> 
> ..and here is a patch that moves tre.scm over as well. The code is
> unchanged apart from license: prefix.

When moving packages around, all users of the package need to have their
module imports updated. Copyright attribution must be carefully handled.
And merging the various *-updates branches into master and vice versa
becomes more complicated and prone to error.

I *think* this patch does it right. And it will really grate on my sense
of aesthetics to have both regex.scm and tre.scm. But, in general, do we
want to make this change? What does everyone think?
Alex Kost - Aug. 19, 2016, 7:35 a.m.
Leo Famulari (2016-08-18 23:43 +0300) wrote:

> On Wed, Aug 17, 2016 at 04:18:48PM +0100, Marius Bakke wrote:
>> Marius Bakke <mbakke@fastmail.com> writes:
>> 
>> > Leo Famulari <leo@famulari.name> writes:
>> >
>> >> On Mon, Aug 15, 2016 at 02:05:16PM +0100, Marius Bakke wrote:
>> >>> 
>> >>> I wasn't sure where to put this, so went with its own file. It does not
>> >>> fully implement PCRE so pcre.scm seems inappropriate. Perhaps that could
>> >>> be renamed to regex.scm or similar.
>> >>
>> >> The patch looks good, but please put it in regex.scm! But, I don't want
>> >> to move all the regex packages into this new regex module. Perhaps tre,
>> >> since it appears to have no users in our tree.
>> >
>> > Moved to regex.scm. Thanks!
>> 
>> ..and here is a patch that moves tre.scm over as well. The code is
>> unchanged apart from license: prefix.
>
> When moving packages around, all users of the package need to have their
> module imports updated. Copyright attribution must be carefully handled.
> And merging the various *-updates branches into master and vice versa
> becomes more complicated and prone to error.
>
> I *think* this patch does it right. And it will really grate on my sense
> of aesthetics to have both regex.scm and tre.scm. But, in general, do we
> want to make this change? What does everyone think?

I agree that having both 'tre' and 're2' packages in "regex.scm" is the
right thing.  Also I think it would be good to move 'oniguruma' there.

As for "pcre.scm", I would also move its content to "regex.scm".  What
about 'ghc-pcre-light'?  It seems natural to keep it in the same file
with 'pcre'/'pcre2' or should it stay in "haskell.scm"?

But also there are regex libraries for various languages (like
'ghc-regex-posix', 'guile-irregex', 'perl-regexp-common').  I think
these shouldn't be moved.
Marius Bakke - Aug. 19, 2016, 11:37 a.m.
Leo Famulari <leo@famulari.name> writes:

> On Wed, Aug 17, 2016 at 04:18:48PM +0100, Marius Bakke wrote:
>> Marius Bakke <mbakke@fastmail.com> writes:
>> 
>> > Leo Famulari <leo@famulari.name> writes:
>> >
>> >> On Mon, Aug 15, 2016 at 02:05:16PM +0100, Marius Bakke wrote:
>> >>> 
>> >>> I wasn't sure where to put this, so went with its own file. It does not
>> >>> fully implement PCRE so pcre.scm seems inappropriate. Perhaps that could
>> >>> be renamed to regex.scm or similar.
>> >>
>> >> The patch looks good, but please put it in regex.scm! But, I don't want
>> >> to move all the regex packages into this new regex module. Perhaps tre,
>> >> since it appears to have no users in our tree.
>> >
>> > Moved to regex.scm. Thanks!
>> 
>> ..and here is a patch that moves tre.scm over as well. The code is
>> unchanged apart from license: prefix.
>
> When moving packages around, all users of the package need to have their
> module imports updated. Copyright attribution must be carefully handled.
> And merging the various *-updates branches into master and vice versa
> becomes more complicated and prone to error.
>
> I *think* this patch does it right. And it will really grate on my sense
> of aesthetics to have both regex.scm and tre.scm. But, in general, do we
> want to make this change? What does everyone think?

I agree that package moves should be kept at a minimum. Not only does it
make merges difficult, but git log/blame information is also "lost", as
in having to check out pre-move versions to trace the history. It can
also break third-party repositories as I found after tcsh move.

...for this particular package (tre) however, I think a move is
inevitable. It has no in-tree users and will forever stay alone in
tre.scm. It doesn't mean we have to go out of our way to move everything
else though. There are more important things to spend time on right now.
Marius Bakke - Aug. 19, 2016, 12:33 p.m.
Leo Famulari <leo@famulari.name> writes:

> When moving packages around, all users of the package need to have their
> module imports updated. Copyright attribution must be carefully handled.
> And merging the various *-updates branches into master and vice versa
> becomes more complicated and prone to error.

Perhaps it makes sense to schedule package moves just after big merges
(e.g. before cutting a new core-updates branch). Or have them in
separate short-lived branches to give users (and Hydra, considering the
recent tcsh evaluation failure) some time to test and prepare.

My 2 NOK,
Marius
Leo Famulari - Aug. 19, 2016, 10:49 p.m.
On Fri, Aug 19, 2016 at 10:35:14AM +0300, Alex Kost wrote:
> I agree that having both 'tre' and 're2' packages in "regex.scm" is the
> right thing.  Also I think it would be good to move 'oniguruma' there.
> 
> As for "pcre.scm", I would also move its content to "regex.scm".  What
> about 'ghc-pcre-light'?  It seems natural to keep it in the same file
> with 'pcre'/'pcre2' or should it stay in "haskell.scm"?
> 
> But also there are regex libraries for various languages (like
> 'ghc-regex-posix', 'guile-irregex', 'perl-regexp-common').  I think
> these shouldn't be moved.

Your first paragraph made it sound so simple. As your message continued,
it illustrated how complicated this issue can be ;)

BTW, if I had remembered the textutils module, I would probably have put
re2 there.
Alex Kost - Sept. 12, 2016, 1:38 p.m.
Marius Bakke (2016-08-17 16:18 +0100) wrote:

> ..and here is a patch that moves tre.scm over as well. The code is
> unchanged apart from license: prefix.
>
> From 8e673ebd8c68c3a92fa60b56068339c011607752 Mon Sep 17 00:00:00 2001
> From: Marius Bakke <mbakke@fastmail.com>
> Date: Wed, 17 Aug 2016 16:10:15 +0100
> Subject: [PATCH 2/2] gnu: tre: Move to regex.scm.
>
> * gnu/packages/tre.scm (tre): Move from here ...
> * gnu/packages/regex.scm (tre): ... to here.
> * gnu/packages/tre.scm: Delete file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Remove tre.scm.

Sorry for the long delay, I've found this patch is my mail heap :-)
I adjusted the commit message a bit (to avoid duplicating
"* gnu/packages/tre.scm") and committed it as ac8a642.
Thanks!
Marius Bakke - Sept. 12, 2016, 2:09 p.m.
Alex Kost <alezost@gmail.com> writes:

> Marius Bakke (2016-08-17 16:18 +0100) wrote:
>
>> ..and here is a patch that moves tre.scm over as well. The code is
>> unchanged apart from license: prefix.
>>
>> From 8e673ebd8c68c3a92fa60b56068339c011607752 Mon Sep 17 00:00:00 2001
>> From: Marius Bakke <mbakke@fastmail.com>
>> Date: Wed, 17 Aug 2016 16:10:15 +0100
>> Subject: [PATCH 2/2] gnu: tre: Move to regex.scm.
>>
>> * gnu/packages/tre.scm (tre): Move from here ...
>> * gnu/packages/regex.scm (tre): ... to here.
>> * gnu/packages/tre.scm: Delete file.
>> * gnu/local.mk (GNU_SYSTEM_MODULES): Remove tre.scm.
>
> Sorry for the long delay, I've found this patch is my mail heap :-)
> I adjusted the commit message a bit (to avoid duplicating
> "* gnu/packages/tre.scm") and committed it as ac8a642.
> Thanks!

I think it was delayed, because we now have 'regex.scm' and
'textutils.scm'. Perhaps those two should be merged :) 

Thanks,
Marius
Alex Kost - Sept. 13, 2016, 7:44 a.m.
Marius Bakke (2016-09-12 15:09 +0100) wrote:

> Alex Kost <alezost@gmail.com> writes:
>
>> Marius Bakke (2016-08-17 16:18 +0100) wrote:
>>
>>> ..and here is a patch that moves tre.scm over as well. The code is
>>> unchanged apart from license: prefix.
>>>
>>> From 8e673ebd8c68c3a92fa60b56068339c011607752 Mon Sep 17 00:00:00 2001
>>> From: Marius Bakke <mbakke@fastmail.com>
>>> Date: Wed, 17 Aug 2016 16:10:15 +0100
>>> Subject: [PATCH 2/2] gnu: tre: Move to regex.scm.
>>>
>>> * gnu/packages/tre.scm (tre): Move from here ...
>>> * gnu/packages/regex.scm (tre): ... to here.
>>> * gnu/packages/tre.scm: Delete file.
>>> * gnu/local.mk (GNU_SYSTEM_MODULES): Remove tre.scm.
>>
>> Sorry for the long delay, I've found this patch is my mail heap :-)
>> I adjusted the commit message a bit (to avoid duplicating
>> "* gnu/packages/tre.scm") and committed it as ac8a642.
>> Thanks!
>
> I think it was delayed, because we now have 'regex.scm' and
> 'textutils.scm'. Perhaps those two should be merged :)

Perhaps, I don't know; but your patch was fine since it removed a
single-package file which is a good thing.  Merging "regex" and
"textutils" (if it will happen) is a separate thing.

Patch

From 8e673ebd8c68c3a92fa60b56068339c011607752 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Wed, 17 Aug 2016 16:10:15 +0100
Subject: [PATCH 2/2] gnu: tre: Move to regex.scm.

* gnu/packages/tre.scm (tre): Move from here ...
* gnu/packages/regex.scm (tre): ... to here.
* gnu/packages/tre.scm: Delete file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Remove tre.scm.
---
 gnu/local.mk           |  1 -
 gnu/packages/regex.scm | 34 ++++++++++++++++++++++++++++++
 gnu/packages/tre.scm   | 57 --------------------------------------------------
 3 files changed, 34 insertions(+), 58 deletions(-)
 delete mode 100644 gnu/packages/tre.scm

diff --git a/gnu/local.mk b/gnu/local.mk
index a18ed44..ebf2d13 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -342,7 +342,6 @@  GNU_SYSTEM_MODULES =				\
   %D%/packages/tls.scm				\
   %D%/packages/tmux.scm				\
   %D%/packages/tor.scm				\
-  %D%/packages/tre.scm				\
   %D%/packages/tv.scm				\
   %D%/packages/unrtf.scm			\
   %D%/packages/upnp.scm				\
diff --git a/gnu/packages/regex.scm b/gnu/packages/regex.scm
index cea9db8..b34b26d 100644
--- a/gnu/packages/regex.scm
+++ b/gnu/packages/regex.scm
@@ -1,4 +1,6 @@ 
 ;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2014 John Darrington
+;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2016 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -55,3 +57,35 @@ 
 backtracking regular expression engines like those used in PCRE, Perl and
 Python.  It is a C++ library.")
      (license license:bsd-3)))
+
+(define-public tre
+  (package
+    (name "tre")
+    (version "0.8.0")
+    (source
+      (origin
+        (method url-fetch)
+        (uri
+          (string-append "http://laurikari.net/tre/" name "-" version
+                         ".tar.bz2"))
+        (sha256
+          (base32 "0n36cgqys59r2gmb7jzbqiwsy790v8nbxk82d2n2saz0rp145ild"))))
+
+    (build-system gnu-build-system)
+    (arguments
+     `(#:phases (alist-cons-before
+                 'check 'install-locales
+                  (lambda _
+                    ;; The tests require the availability of the
+                    ;; 'en_US.ISO-8859-1' locale.
+                    (setenv "LOCPATH" (getcwd))
+                    (zero? (system* "localedef" "--no-archive"
+                                    "--prefix" (getcwd) "-i" "en_US"
+                                    "-f" "ISO-8859-1" "./en_US.ISO-8859-1")))
+                 %standard-phases)))
+    (synopsis "Approximate regex matching library and agrep utility")
+    (description "Superset of the POSIX regex API, enabling approximate
+matching.  Also ships a version of the agrep utility which behaves similar to
+grep but features inexact matching.")
+    (home-page "http://laurikari.net/tre")
+    (license license:bsd-2)))
diff --git a/gnu/packages/tre.scm b/gnu/packages/tre.scm
deleted file mode 100644
index 721a350..0000000
--- a/gnu/packages/tre.scm
+++ /dev/null
@@ -1,57 +0,0 @@ 
-;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2014 John Darrington
-;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
-;;;
-;;; This file is part of GNU Guix.
-;;;
-;;; GNU Guix is free software; you can redistribute it and/or modify it
-;;; under the terms of the GNU General Public License as published by
-;;; the Free Software Foundation; either version 3 of the License, or (at
-;;; your option) any later version.
-;;;
-;;; GNU Guix is distributed 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.  See the
-;;; GNU General Public License for more details.
-;;;
-;;; You should have received a copy of the GNU General Public License
-;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
-
-(define-module (gnu packages tre)
-  #:use-module (gnu packages)
-  #:use-module (guix packages)
-  #:use-module (guix download)
-  #:use-module (guix build-system gnu)
-  #:use-module (guix licenses))
-
-(define-public tre
-  (package
-    (name "tre")
-    (version "0.8.0")
-    (source
-      (origin
-        (method url-fetch)
-        (uri
-          (string-append "http://laurikari.net/tre/" name "-" version
-                         ".tar.bz2"))
-        (sha256
-          (base32 "0n36cgqys59r2gmb7jzbqiwsy790v8nbxk82d2n2saz0rp145ild"))))
-
-    (build-system gnu-build-system)
-    (arguments
-     `(#:phases (alist-cons-before
-                 'check 'install-locales
-                  (lambda _
-                    ;; The tests require the availability of the
-                    ;; 'en_US.ISO-8859-1' locale.
-                    (setenv "LOCPATH" (getcwd))
-                    (zero? (system* "localedef" "--no-archive"
-                                    "--prefix" (getcwd) "-i" "en_US"
-                                    "-f" "ISO-8859-1" "./en_US.ISO-8859-1")))
-                 %standard-phases)))
-    (synopsis "Approximate regex matching library and agrep utility")
-    (description "Superset of the POSIX regex API, enabling approximate
-matching.  Also ships a version of the agrep utility which behaves similar to
-grep but features inexact matching.")
-    (home-page "http://laurikari.net/tre")
-    (license bsd-2)))
-- 
2.9.2