code review 4412045: make: prevent rm provoking 'text file busy' errors.

make: prevent rm provoking 'text file busy' errors.

Trivial patch to stop intermediate rm'ing of binaries stopping build.

[lstoakes, 2011-04-14 17:21:32]

Hello rsc1 (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2011-04-14 17:37:08]

Gross.

If anything I'd prefer a custom rm command that only ignored 'text file
busy' errors from the unlink system call, and perhaps only then on
known-buggy filesystems.

But I'd hate to paper over otherwise-interesting failures with this, just
for one weird buggy filesystem.

On Thu, Apr 14, 2011 at 10:21 AM, <lstoakes@gmail.com> wrote:

> Reviewers: rsc1,
>
> Message:
> Hello rsc1 (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> make: prevent rm provoking 'text file busy' errors.
>
> Trivial patch to stop intermediate rm'ing of binaries stopping build.
>
> Please review this at http://codereview.appspot.com/4412045/
>
> Affected files:
>  M src/Make.common
>
>
> Index: src/Make.common
> ===================================================================
> --- a/src/Make.common
> +++ b/src/Make.common
> @@ -3,16 +3,16 @@
>  # license that can be found in the LICENSE file.
>
>  clean:
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  install.clean: install
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  test.clean: test
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  testshort.clean: testshort
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  %.make:
>        $(MAKE) -C $* install
>
>
>

[rsc1, 2011-04-14 17:43:07]

Please leave the clean rule alone.

The others - install.clean, test.clean, and testshort.clean - are
optimizations so they can stay and we can ignore the error.

[lstoakes, 2011-04-14 19:43:17]

On Thursday, 14 April 2011, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> Gross.

I know :) it felt like quite the papering.


> If anything I'd prefer a custom rm command that only ignored 'text file busy'
errors from the unlink system call, and perhaps only then on known-buggy
filesystems.

I am happy to amend the CL to this, specifically targeted to a list of
known buggy systems.

>
> But I'd hate to paper over otherwise-interesting failures with this, just for
one weird buggy filesystem.
> On Thu, Apr 14, 2011 at 10:21 AM,  <lstoakes@gmail.com> wrote:
> Reviewers: rsc1,
>
> Message:
> Hello rsc1 (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> make: prevent rm provoking 'text file busy' errors.
>
> Trivial patch to stop intermediate rm'ing of binaries stopping build.
>
> Please review this at http://codereview.appspot.com/4412045/
>
> Affected files:
>   M src/Make.common
>
>
> Index: src/Make.common
> ===================================================================
> --- a/src/Make.common
> +++ b/src/Make.common
> @@ -3,16 +3,16 @@
>  # license that can be found in the LICENSE file.
>
>  clean:
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  install.clean: install
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  test.clean: test
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  testshort.clean: testshort
> -       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES)
> +       rm -rf *.o *.a *.[$(OS)] [$(OS)].out $(CLEANFILES) || true
>
>  %.make:
>         $(MAKE) -C $* install
>
>
>
>
>

-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[rsc1, 2011-04-14 20:12:30]

See my earlier reply.
That's all you need to change.
Let's not overdo this.

[lstoakes, 2011-04-14 20:41:38]

On Thursday, 14 April 2011, Russ Cox <rsc@google.com> wrote:
> See my earlier reply.
> That's all you need to change.
> Let's not overdo this.
>

So we're good just to ignore the .clean errors as proposed in the CL?

-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[rsc1, 2011-04-14 20:47:18]

> So we're good just to ignore the .clean errors as proposed in the CL?

Yes, please make that change.

[lstoakes, 2011-04-15 08:52:13]

Hello rsc1, bradfitzgo (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-04-15 12:25:48]

*** Submitted as http://code.google.com/p/go/source/detail?r=39c5a72b1189 ***

make: prevent rm provoking 'text file busy' errors.

Trivial patch to stop intermediate rm'ing of binaries stopping build.

R=rsc1, bradfitzgo, rsc
CC=golang-dev
http://codereview.appspot.com/4412045

Committer: Russ Cox <rsc@golang.org>

[rsc1, 2011-04-15 12:25:57]

LGTM