[PATCH] Clean all .go in clean-go

[PATCH] Clean all .go in clean-go

[Eric Bavier]

From: Eric Bavier <bavier@member.fsf.org>

I encountered a runtime error recently while running `guix system
reconfigure`.  Thinking this might be because of an ABI break I ran `make clean-go && make` before trying again, with the same result.

It turns out a module had been renamed, in this case fish.scm to shells.scm,
but I had overlooked this and failed to update the list of modules in my
config.scm's (use-package-modules ...) statement.  However, I still had a
stale fish.go sitting in my build directory, which `make clean-go` had failed
to clean up, and guix happily loaded it.

I believe the following patch is an appropriate way to avoid such errors in
the future.

Eric Bavier (1):
  build: Clean all .go files in clean-go.

 Makefile.am | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.9.3

[PATCH] build: Clean all .go files in clean-go.

[Eric Bavier]

From: Eric Bavier <bavier@member.fsf.org>

* Makefile.am (clean-go): Use wildcards instead of a list of files.
---
 Makefile.am | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 165dfe9..36d1d78 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -204,9 +204,10 @@ srfi/srfi-37.scm: srfi/srfi-37.scm.in
 
 endif INSTALL_SRFI_37
 
-# Handy way to remove the .go files without removing all the rest.
+# Handy way to remove the .go files without removing all the rest.  Use
+# wildcards so that any stale .go files from renamed modules are also cleaned.
 clean-go:
-	-$(RM) -f $(GOBJECTS)
+	-$(RM) -f $(addsuffix *.go,$(sort $(dir $(GOBJECTS))))
 
 
 # Test extensions; has to be unconditional.
-- 
2.9.3

Re: [PATCH] Clean all .go in clean-go

[Mathieu Lirzin]

Hello,

Eric Bavier <ericbavier@openmailbox.org> writes:

> From: Eric Bavier <bavier@member.fsf.org>
>
> I encountered a runtime error recently while running `guix system
> reconfigure`.  Thinking this might be because of an ABI break I ran `make clean-go && make` before trying again, with the same result.
>
> It turns out a module had been renamed, in this case fish.scm to shells.scm,
> but I had overlooked this and failed to update the list of modules in my
> config.scm's (use-package-modules ...) statement.  However, I still had a
> stale fish.go sitting in my build directory, which `make clean-go` had failed
> to clean up, and guix happily loaded it.
>
> I believe the following patch is an appropriate way to avoid such errors in
> the future.

AIUI the main problem is that the API for defining "config.scm" is not
stable because of the package modules renames.  Since package names are
more stable, I think that configuration files should import (gnu
packages) and use 'specification->package' when possible to resolve
packages, instead of relying on the module names.  Maybe we should fix
the "config.scm" documentation?

In regards of the .go files remaining in the build directory, I agree
that this is not good, however I don't think it is worth trying to fix
this issue which equally applies to every file generated by Make.  Using
wildcards can be tempting in such cases but it can lead to accidental
file deletions which is worse IMO.  As a consequence I would prefer
keeping the current 'clean-go' rule.

-- 
Mathieu Lirzin

Re: [PATCH] Clean all .go in clean-go

[David Craven]

> Using wildcards can be tempting in such cases but it can lead to accidental file deletions which is worse IMO.

If you name what ever file *.go inside of your guix source tree, it's
kind of your on fault.

> Thinking this might be because of an ABI break I ran `make clean-go && make` before trying again, with the same result.

mmh, I usually just do make clean && make, do you save that much time
using make clean-go?

Re: [PATCH] Clean all .go in clean-go

[Ludovic Courtès]

Mathieu Lirzin <mthl@gnu.org> skribis:

> Eric Bavier <ericbavier@openmailbox.org> writes:
>
>> From: Eric Bavier <bavier@member.fsf.org>
>>
>> I encountered a runtime error recently while running `guix system
>> reconfigure`.  Thinking this might be because of an ABI break I ran `make clean-go && make` before trying again, with the same result.
>>
>> It turns out a module had been renamed, in this case fish.scm to shells.scm,
>> but I had overlooked this and failed to update the list of modules in my
>> config.scm's (use-package-modules ...) statement.  However, I still had a
>> stale fish.go sitting in my build directory, which `make clean-go` had failed
>> to clean up, and guix happily loaded it.
>>
>> I believe the following patch is an appropriate way to avoid such errors in
>> the future.
>
> AIUI the main problem is that the API for defining "config.scm" is not
> stable because of the package modules renames.  Since package names are
> more stable, I think that configuration files should import (gnu
> packages) and use 'specification->package' when possible to resolve
> packages, instead of relying on the module names.  Maybe we should fix
> the "config.scm" documentation?

Look, the fine manual already mentions it (info "(guix) Using the
Configuration System").  :-)

I’ve also been hit by what Eric describes though.  However, the indented
use case is that people use ‘guix pull’, in which case they cannot have
this sort of problem; at worse they get “no code for module” or similar.
When I use ./pre-inst-env, I feel like I have my wizard hat on and no
safety belt.  ;-)

> In regards of the .go files remaining in the build directory, I agree
> that this is not good, however I don't think it is worth trying to fix
> this issue which equally applies to every file generated by Make.  Using
> wildcards can be tempting in such cases but it can lead to accidental
> file deletions which is worse IMO.  As a consequence I would prefer
> keeping the current 'clean-go' rule.

I sympathize with that.

Ludo’.

Re: [PATCH] Clean all .go in clean-go

[Eric Bavier]

On Thu, 01 Sep 2016 14:37:58 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Mathieu Lirzin <mthl@gnu.org> skribis:
> 
> > Eric Bavier <ericbavier@openmailbox.org> writes:
> >  
> >> From: Eric Bavier <bavier@member.fsf.org>
> >>
> >> I encountered a runtime error recently while running `guix system
> >> reconfigure`.  Thinking this might be because of an ABI break I ran `make clean-go && make` before trying again, with the same result.
> >>
> >> It turns out a module had been renamed, in this case fish.scm to shells.scm,
> >> but I had overlooked this and failed to update the list of modules in my
> >> config.scm's (use-package-modules ...) statement.  However, I still had a
> >> stale fish.go sitting in my build directory, which `make clean-go` had failed
> >> to clean up, and guix happily loaded it.
> >>
> >> I believe the following patch is an appropriate way to avoid such errors in
> >> the future.  
> >
> > AIUI the main problem is that the API for defining "config.scm" is not
> > stable because of the package modules renames.  Since package names are
> > more stable, I think that configuration files should import (gnu
> > packages) and use 'specification->package' when possible to resolve
> > packages, instead of relying on the module names.  Maybe we should fix
> > the "config.scm" documentation?  
> 
> Look, the fine manual already mentions it (info "(guix) Using the
> Configuration System").  :-)
> 
> I’ve also been hit by what Eric describes though.  However, the indented
> use case is that people use ‘guix pull’, in which case they cannot have
> this sort of problem; at worse they get “no code for module” or similar.

Sure, but we also regularly recommend users to maintain a git checkout.

> When I use ./pre-inst-env, I feel like I have my wizard hat on and no
> safety belt.  ;-)

This is in fact what I do most times.

> 
> > In regards of the .go files remaining in the build directory, I agree
> > that this is not good, however I don't think it is worth trying to fix
> > this issue which equally applies to every file generated by Make.  Using
> > wildcards can be tempting in such cases but it can lead to accidental
> > file deletions which is worse IMO.  As a consequence I would prefer
> > keeping the current 'clean-go' rule.  
> 
> I sympathize with that.

How about simply printing  a warning if there are any .go files laying
around after a `make clean` or `make clean-go`?

`~Eric

Re: [PATCH] Clean all .go in clean-go

[Ludovic Courtès]

Eric Bavier <ericbavier@openmailbox.org> skribis:

> On Thu, 01 Sep 2016 14:37:58 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:

[...]

>> > In regards of the .go files remaining in the build directory, I agree
>> > that this is not good, however I don't think it is worth trying to fix
>> > this issue which equally applies to every file generated by Make.  Using
>> > wildcards can be tempting in such cases but it can lead to accidental
>> > file deletions which is worse IMO.  As a consequence I would prefer
>> > keeping the current 'clean-go' rule.  
>> 
>> I sympathize with that.
>
> How about simply printing  a warning if there are any .go files laying
> around after a `make clean` or `make clean-go`?

Sure, why not.

Ludo’.

Re: [PATCH] Clean all .go in clean-go

[Eric Bavier]

[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]

On Fri, 02 Sep 2016 14:42:27 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Eric Bavier <ericbavier@openmailbox.org> skribis:
> 
> > On Thu, 01 Sep 2016 14:37:58 +0200
> > ludo@gnu.org (Ludovic Courtès) wrote:  
> 
> [...]
> 
> >> > In regards of the .go files remaining in the build directory, I agree
> >> > that this is not good, however I don't think it is worth trying to fix
> >> > this issue which equally applies to every file generated by Make.  Using
> >> > wildcards can be tempting in such cases but it can lead to accidental
> >> > file deletions which is worse IMO.  As a consequence I would prefer
> >> > keeping the current 'clean-go' rule.    
> >> 
> >> I sympathize with that.  
> >
> > How about simply printing  a warning if there are any .go files laying
> > around after a `make clean` or `make clean-go`?  
> 
> Sure, why not.

So, with the attached patch, I get the following output after `make
clean-go`:

warning: stray .go files: ./guix/scripts/import/cpan.go ./gnu/services/dmd.go
./gnu/system/linux.go ./gnu/packages/yasm.go ./gnu/packages/cursynth.go 
./gnu/packages/lightning.go ./gnu/packages/doxygen.go ./gnu/packages/tre.go 
./gnu/packages/asciidoc.go ./gnu/packages/texlive.go ./gnu/packages/i3.go 
./gnu/packages/fish.go ./gnu/packages/slim.go ./gnu/packages/tcsh.go 
./gnu/packages/zsh.go ./gnu/packages/lsh.go ./gnu/packages/rc.go 
./gnu/packages/openssl.go ./gnu/packages/aria2.go ./gnu/packages/gdbm.go 
./gnu/packages/gnutls.go ./gnu/packages/grue-hunter.go ./gnu/packages/aarddict.go

Maybe this means that I've not been doing due diligence in keeping my
builddir clean, or maybe its just the result of developing on guix for
so long.

Another issue that I thought of that's posed by these stray .go files,
which isn't only a problem when acting as a wizard: introducing a new
module import when adding a package, only to have that module
moved/renamed before pushing your change.  I suppose it is a corner
case, but it did happen to me recently, in 2e3f18511, which 宋文武
kindly fixed in 19b2ea1b6. The problem was that between the time I
originally packaged tomb and the time I pushed the commit,
gnu/packages/zsh.scm was consolidated into gnu/packages/shells.scm.  I
had done a `make clean-go && make` before pushing, as I always do, but
since clean-go didn't remove gnu/packages/zsh.go the subsequent `make`
didn't complain of a missing module.

`~Eric

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-build-clean-go-warn-about-stray-.go-files.patch --]
[-- Type: text/x-patch, Size: 881 bytes --]

From ffe1710d5a91cd9906f2419d21944b1375b1b842 Mon Sep 17 00:00:00 2001
From: Eric Bavier <bavier@member.fsf.org>
Date: Thu, 15 Sep 2016 23:31:41 -0500
Subject: [PATCH] build: clean-go: warn about stray .go files.

* Makefile.am (clean-go): Warn of .go files remaining in builddir.
---
 Makefile.am | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index f9fe141..43a33c8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -208,6 +208,12 @@ endif INSTALL_SRFI_37
 # Handy way to remove the .go files without removing all the rest.
 clean-go:
 	-$(RM) -f $(GOBJECTS)
+	@find . -name '*.go' -print | \
+	  if test -t 1; then \
+	    xargs -r echo -e "\033[31mwarning:\033[0m stray .go files:"; \
+	  else \
+	    xargs -r echo "warning: stray .go files:"; \
+	  fi
 
 
 # Test extensions; has to be unconditional.
-- 
2.9.2

Re: [PATCH] Clean all .go in clean-go

[Leo Famulari]

On Fri, Sep 16, 2016 at 12:15:40AM -0500, Eric Bavier wrote:
> Another issue that I thought of that's posed by these stray .go files,
> which isn't only a problem when acting as a wizard: introducing a new
> module import when adding a package, only to have that module
> moved/renamed before pushing your change.  I suppose it is a corner
> case, but it did happen to me recently, in 2e3f18511, which 宋文武
> kindly fixed in 19b2ea1b6. The problem was that between the time I
> originally packaged tomb and the time I pushed the commit,
> gnu/packages/zsh.scm was consolidated into gnu/packages/shells.scm.  I
> had done a `make clean-go && make` before pushing, as I always do, but
> since clean-go didn't remove gnu/packages/zsh.go the subsequent `make`
> didn't complain of a missing module.

I accidentally did this to myself in c41d97bed6 (gnu: mupdf: Update to
1.9a.).

Re: [PATCH] Clean all .go in clean-go

[Ludovic Courtès]

Hi,

Eric Bavier <ericbavier@openmailbox.org> skribis:

> So, with the attached patch, I get the following output after `make
> clean-go`:
>
> warning: stray .go files: ./guix/scripts/import/cpan.go ./gnu/services/dmd.go
> ./gnu/system/linux.go ./gnu/packages/yasm.go ./gnu/packages/cursynth.go 
> ./gnu/packages/lightning.go ./gnu/packages/doxygen.go ./gnu/packages/tre.go 
> ./gnu/packages/asciidoc.go ./gnu/packages/texlive.go ./gnu/packages/i3.go 
> ./gnu/packages/fish.go ./gnu/packages/slim.go ./gnu/packages/tcsh.go 
> ./gnu/packages/zsh.go ./gnu/packages/lsh.go ./gnu/packages/rc.go 
> ./gnu/packages/openssl.go ./gnu/packages/aria2.go ./gnu/packages/gdbm.go 
> ./gnu/packages/gnutls.go ./gnu/packages/grue-hunter.go ./gnu/packages/aarddict.go

Looks good, surely an improvement!

> Another issue that I thought of that's posed by these stray .go files,
> which isn't only a problem when acting as a wizard: introducing a new
> module import when adding a package, only to have that module
> moved/renamed before pushing your change.  I suppose it is a corner
> case, but it did happen to me recently, in 2e3f18511, which 宋文武
> kindly fixed in 19b2ea1b6. The problem was that between the time I
> originally packaged tomb and the time I pushed the commit,
> gnu/packages/zsh.scm was consolidated into gnu/packages/shells.scm.  I
> had done a `make clean-go && make` before pushing, as I always do, but
> since clean-go didn't remove gnu/packages/zsh.go the subsequent `make`
> didn't complain of a missing module.

Indeed, that’s happened to me before.

> From ffe1710d5a91cd9906f2419d21944b1375b1b842 Mon Sep 17 00:00:00 2001
> From: Eric Bavier <bavier@member.fsf.org>
> Date: Thu, 15 Sep 2016 23:31:41 -0500
> Subject: [PATCH] build: clean-go: warn about stray .go files.
>
> * Makefile.am (clean-go): Warn of .go files remaining in builddir.

LGTM, thank you!

Ludo’.

Re: [PATCH] Clean all .go in clean-go

[myglc2]

On 09/16/2016 at 01:15 Eric Bavier writes:

> On Fri, 02 Sep 2016 14:42:27 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> Eric Bavier <ericbavier@openmailbox.org> skribis:
>> 
>> > On Thu, 01 Sep 2016 14:37:58 +0200
>> > ludo@gnu.org (Ludovic Courtès) wrote:  
>> 
>> [...]
>> 
>> >> > In regards of the .go files remaining in the build directory, I agree
>> >> > that this is not good, however I don't think it is worth trying to fix
>> >> > this issue which equally applies to every file generated by Make.  Using
>> >> > wildcards can be tempting in such cases but it can lead to accidental
>> >> > file deletions which is worse IMO.  As a consequence I would prefer
>> >> > keeping the current 'clean-go' rule.    
>> >> 
>> >> I sympathize with that.  
>> >
>> > How about simply printing  a warning if there are any .go files laying
>> > around after a `make clean` or `make clean-go`?  
>> 
>> Sure, why not.
>
> So, with the attached patch, I get the following output after `make
> clean-go`:
>
> warning: stray .go files: ./guix/scripts/import/cpan.go ./gnu/services/dmd.go
> ./gnu/system/linux.go ./gnu/packages/yasm.go ./gnu/packages/cursynth.go 
> ./gnu/packages/lightning.go ./gnu/packages/doxygen.go ./gnu/packages/tre.go 
> ./gnu/packages/asciidoc.go ./gnu/packages/texlive.go ./gnu/packages/i3.go 
> ./gnu/packages/fish.go ./gnu/packages/slim.go ./gnu/packages/tcsh.go 
> ./gnu/packages/zsh.go ./gnu/packages/lsh.go ./gnu/packages/rc.go 
> ./gnu/packages/openssl.go ./gnu/packages/aria2.go ./gnu/packages/gdbm.go 
> ./gnu/packages/gnutls.go ./gnu/packages/grue-hunter.go ./gnu/packages/aarddict.go
>
> Maybe this means that I've not been doing due diligence in keeping my
> builddir clean, or maybe its just the result of developing on guix for
> so long.

Hi, I would like to request that you revisit the disposition of stray
.go files:

Grated, my request is subjective: I am primarily a guix user. But I want
_total artistic control_ so I do my guix upgrades using 'git pull; make'
in order to have editable guix source handy, should I be inspired to
make a change. In practice I do 'git pull; make' every few weeks. I
seldom actually submit patches, but sometimes I mess with packages for
my own ends.

<rant> As I understand it, my mode of use is not "supported". This
mystifies me because I actually think it is in stronger solidarity with
the FWF source mantra than guix pull, which gives read-only source in
the store which is not, by default, easily editable and so is just plain
annoying to moi! If Guix wants to encourage users to look at source,
find problems, and submit patches, I think you should encourage them to
use git checkout the same way the "real developers" do </rant>

Anyway, in my case I can't imagine why I would ever want "stray .go
files" kicking around my guix working tree. Can you?

Previously I was bitten when the guix API changed and was advised to do
'make clean-go' to "fix" the apparent bugs that this generated for
me. Having a reasonably powerful machine, I have adopted the practice of
routinely doing 'git pull; make clean-go; ./bootstrap ;./configure
--localstatedir=/var' to avoid such problems in the future.

Recently I was bitten by a stray .go file (bug#29072). In that situation
I would have been better off if 'make clean-go' nuked all the .go
files. Admittedly, I failed to notice the subtle stray .go warning, but,
in my defense, I was assuming that 'make clean-go' would just nuke all
the .go files so I wasn't expecting or looking for such a warning.

Maybe I am missing something here. Are there situations where it is
truly desirable for the guix 'make clean-go' to preserve .go files for
which no source exists? If not, maybe 'clean-go' should delete the stray
files by default, and issue a warning saying which stray files were
deleted.

Alternatively... how about adding a new make target that also removes
the stray .go files?

- George

Re: [PATCH] Clean all .go in clean-go

[Vincent Legoll]

Hello,

> Alternatively... how about adding a new make target that also removes
> the stray .go files?

what about:

make mrproper-go

Sorry, couldn't resist...

-- 
Vincent Legoll