unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] libnotmuch: fix typo in CLEAN setting, add file
@ 2011-06-24 10:53 david
  2011-06-24 17:51 ` Carl Worth
  0 siblings, 1 reply; 4+ messages in thread
From: david @ 2011-06-24 10:53 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

- c0961e6 introduced a missing slash and
- cdf1c70a created a file and neglected to add it to clean

The former seems to have been harmless, so maybe someone (Carl?) can
check if $(dir)/$(LIBNAME) really needs to be removed?
---
 lib/Makefile.local |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/lib/Makefile.local b/lib/Makefile.local
index a33ba34..acf257a 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -103,4 +103,6 @@ install-$(dir): $(dir)/$(LIBNAME)
 	$(LIBRARY_INSTALL_POST_COMMAND)
 
 SRCS  := $(SRCS) $(libnotmuch_c_srcs) $(libnotmuch_cxx_srcs)
-CLEAN := $(CLEAN) $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME) $(dir)$(LIBNAME) libnotmuch.a notmuch.aux notmuch.sym
+CLEAN += $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME)
+CLEAN += $(dir)/$(LIBNAME) libnotmuch.a notmuch.aux notmuch.sym 
+CLEAN += $(dir)/notmuch.h.gch
-- 
1.7.5.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] libnotmuch: fix typo in CLEAN setting, add file
  2011-06-24 10:53 [PATCH] libnotmuch: fix typo in CLEAN setting, add file david
@ 2011-06-24 17:51 ` Carl Worth
  2011-06-25 12:45   ` [PATCH] libnotmuch: fix typos " david
  0 siblings, 1 reply; 4+ messages in thread
From: Carl Worth @ 2011-06-24 17:51 UTC (permalink / raw)
  To: david, notmuch; +Cc: David Bremner

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

On Fri, 24 Jun 2011 07:53:44 -0300, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>

Hi David,

Thanks for the fix. A couple of nit-picky comments on the commit itself
first:

> - c0961e6 introduced a missing slash and
> - cdf1c70a created a file and neglected to add it to clean

I'd put a little more detail in the commit message here:

- c0961e6 introduced a missing slash $(dir)$(LIBNAME)
- cdf1c70a created $(dir)/notmuch.h.gch and neglected to add it to clean

> The former seems to have been harmless, so maybe someone (Carl?) can
> check if $(dir)/$(LIBNAME) really needs to be removed?

This auxiliary text should be below the "---" line in the email so that
it won't be in the commit message after I do "git am". (As is, I'd have
to do extra work to "git commit --amend" this text away).

As for the actual question, yes, $(dir)/$(LIBNAME) is supposed to be
cleaned and is not getting cleaned. I've got libnotmuch.so.1.{1,2,3,4}.0
all sitting around never cleaned.

> -CLEAN := $(CLEAN) $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME) $(dir)$(LIBNAME) libnotmuch.a notmuch.aux notmuch.sym
> +CLEAN += $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME)
> +CLEAN += $(dir)/$(LIBNAME) libnotmuch.a notmuch.aux notmuch.sym 

Meanwhile, I notice now that "libnotmuch.a" is also wrong, (should be
"$(dir)/libnotmuch.a).

Clearly we could use some testing of our "make clean" target to ensure
it actually works. Does the Debian stuff not test anything like this?
(Maybe I'm thinking of typical testing done by autoconf/automake's "make
distcheck" that we haven't replicated yet.)

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] libnotmuch: fix typos in CLEAN setting, add file
  2011-06-24 17:51 ` Carl Worth
@ 2011-06-25 12:45   ` david
  2011-06-28 18:55     ` Carl Worth
  0 siblings, 1 reply; 4+ messages in thread
From: david @ 2011-06-25 12:45 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

- c0961e6 introduced a missing slash between $(dir)$(LIBNAME) and missing
  $(dir) in front of libnotmuch.a
- cdf1c70a created a file $(dir)/notmuch.h.gch and neglected to
  add it to CLEAN
---

Here is an updated version. I'm not sure the best way to do a test of
the cleaning; maybe we should ship a MANIFEST file containing the
output of git ls-files. I'm not sure how much churn this would cause
in git.  Perhaps it could be treated like version, and generated from
git if possible. In any case I guess this couldn't really be part of
our regular test suite, because all the other tests would fail ;).

 lib/Makefile.local |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/lib/Makefile.local b/lib/Makefile.local
index a33ba34..eaa8af4 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -103,4 +103,6 @@ install-$(dir): $(dir)/$(LIBNAME)
 	$(LIBRARY_INSTALL_POST_COMMAND)
 
 SRCS  := $(SRCS) $(libnotmuch_c_srcs) $(libnotmuch_cxx_srcs)
-CLEAN := $(CLEAN) $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME) $(dir)$(LIBNAME) libnotmuch.a notmuch.aux notmuch.sym
+CLEAN += $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME)
+CLEAN += $(dir)/$(LIBNAME) $(dir)/libnotmuch.a notmuch.aux notmuch.sym
+CLEAN += $(dir)/notmuch.h.gch
-- 
1.7.5.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] libnotmuch: fix typos in CLEAN setting, add file
  2011-06-25 12:45   ` [PATCH] libnotmuch: fix typos " david
@ 2011-06-28 18:55     ` Carl Worth
  0 siblings, 0 replies; 4+ messages in thread
From: Carl Worth @ 2011-06-28 18:55 UTC (permalink / raw)
  To: david, notmuch; +Cc: David Bremner

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

On Sat, 25 Jun 2011 09:45:58 -0300, david@tethera.net wrote:
> Here is an updated version.

Thanks. I've committed this now, (waiting to be pushed until I fix my
build---the symbols stuff---so I can actually run "make test" again).

> I'm not sure the best way to do a test of
> the cleaning; maybe we should ship a MANIFEST file containing the
> output of git ls-files. I'm not sure how much churn this would cause
> in git.  Perhaps it could be treated like version, and generated from
> git if possible.

Definitely wouldn't want a generated file under revision control. It
would be possible to just run "git ls-files" if we're in a git
repository and skip the test otherwise. Or...

> In any case I guess this couldn't really be part of
> our regular test suite, because all the other tests would fail ;).

Heh, that would be a bad failure mode. :-)

The typical way to do this (as far as I understand) is as part of "make
distcheck". The idea there is to make a tar file; verify that the tar
file can be unpacked, configured, built, and installed; and then to
verify that "make distclean" returns the directory to the same state as
just after having unpacked the tar file.

I'm not really all that concerned about it though. We don't add/remove
files all that often, and it's generally not too bad of a failure mode
if "make clean" isn't perfect.

So if someone added testing for this, that would be fine, but it's not a
high priority for me.

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-06-28 18:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-24 10:53 [PATCH] libnotmuch: fix typo in CLEAN setting, add file david
2011-06-24 17:51 ` Carl Worth
2011-06-25 12:45   ` [PATCH] libnotmuch: fix typos " david
2011-06-28 18:55     ` Carl Worth

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).