Automake file bug fix - src/leveldb/ and src/leveldb.cpp problem #3211

This is about the files that are included in a distribution that is created with "make dist"?

I think * is too general, it's better to be somewhat more specific about the type of files to include (ie, *.cc, *.h) otherwise built objects and libraries and other temporary stuff get included in the distribution too.

To my understanding the previous version did the same thing--copied the leveldb folder recursively.

On Nov 6, 2013, at 9:14 AM, "Wladimir J. van der Laan" notifications@github.com wrote:

In src/Makefile.am:

@@ -81,7 +81,7 @@ CLEANFILES = leveldb/libleveldb.a leveldb/libmemenv.a *.gcda *.gcno

DISTCLEANFILES = obj/build.h

-EXTRA_DIST = leveldb Makefile.include +EXTRA_DIST = leveldb/* Makefile.include This is about the files that are included in a distribution that is created with "make dist"?

I think * is too general, it's better to be somewhat more specific about the type of files to include (ie, *.cc, *.h) otherwise built objects and libraries and other temporary stuff get included in the distribution too.

— Reply to this email directly or view it on GitHub.

Hmm If this is a recursive copy (and not just one level deep), I don't understand why the /* is needed.

Please see the linked issue from the original post.

On Nov 6, 2013, at 12:42 PM, "Wladimir J. van der Laan" notifications@github.com wrote:

In src/Makefile.am:

@@ -81,7 +81,7 @@ CLEANFILES = leveldb/libleveldb.a leveldb/libmemenv.a *.gcda *.gcno

DISTCLEANFILES = obj/build.h

-EXTRA_DIST = leveldb Makefile.include +EXTRA_DIST = leveldb/* Makefile.include Hmm If this is a recursive copy (and not just one level deep), I don't understand why the /* is needed.

— Reply to this email directly or view it on GitHub.

Yes, I've read that issue, it describes the problem but not why this solution works -- the problem is that "leveldb" invoked a rule called "leveldb" and does not include the actual directory recursively? And adding /* works around this? @theuni you're the automake wizard, do you know what's going on here?

Yea, looks like it's this:

leveldb.cpp is touched, so it checks to see what needs to be rebuilt. Possible options are leveldb.o, leveldb.exe, leveldb, etc. It finds that leveldb (the folder) is older than leveldb.cpp, so it must be "rebuilt". The implicit rule tries to build 'leveldb' (a binary) from leveldb.cpp and obviously fails.

distdir depends on all distributed files:

distdir: $(DISTFILES)

Changing 'leveldb' to 'leveldb/*' (or anything that's not exactly 'leveldb') in $EXTRA_DIST removes that dependency from $DISTFILES, so it removes the target as well. That only addresses the symptom though, so I'd rather not go that route.

Possible fixes:

• rename leveldb.cpp (my suggestion. This is likely to cause similar head-scratchers in the future otherwise.)
• rename leveldb folder. Obviously unrealistic.
• add an empty leveldb rule to nullify the implicit one:

leveldb: 

That tells make to do nothing when leveldb needs to be 'rebuilt', rather than using the implicit rule of using the .cpp of the same name. It will be 'rebuilt' with each build, but it's empty so it'd be an invisible no-op. Assuming that works as intended, it'd be my second choice.

NACK on renaming-- that will just cause us headaches merging upstream changes.

Empty rule (with a comment-- we can add a comment, right?) sounds like the correct fix.

Renaming leveldb.cpp should be fine though.

ACK on renaming leveldb.cpp

Ignore that idiot gavin, he thought there was a leveldb/leveldb.cpp ....

Trailing space.

Missed the actual ifdef/define renames.

Nit: Can you update this to // Copyright (c) 2012-2013 The Bitcoin developers and add a new-line below the license header :). And check if the .cpp also needs 2013 please.