New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crystal clean make clean #7504

Merged
merged 1 commit into from Feb 15, 2016

Conversation

Projects
None yet
3 participants
@paveljanik
Contributor

paveljanik commented Feb 10, 2016

Fixes #7501.

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Feb 10, 2016

Contributor

@theuni Please have a look please.

Contributor

paveljanik commented Feb 10, 2016

@theuni Please have a look please.

@laanwj laanwj added the Build system label Feb 10, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 10, 2016

Member

utACK

Member

laanwj commented Feb 10, 2016

utACK

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Feb 10, 2016

Member

@paveljanik Please don't use wildcards unless absolutely necessary, especially when deleting.

You should be able to simply:

CLEANFILES += $(EXTRA_LIBRARIES)

There's no need to clean qt manually, as it's not third-party.

Member

theuni commented Feb 10, 2016

@paveljanik Please don't use wildcards unless absolutely necessary, especially when deleting.

You should be able to simply:

CLEANFILES += $(EXTRA_LIBRARIES)

There's no need to clean qt manually, as it's not third-party.

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Feb 10, 2016

Contributor

@theuni But make clean doesn't clean in qt dir as @laanwj reported...
Will test $(EXTRA_LIBRARIES)...

Contributor

paveljanik commented Feb 10, 2016

@theuni But make clean doesn't clean in qt dir as @laanwj reported...
Will test $(EXTRA_LIBRARIES)...

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Feb 10, 2016

Contributor

Works much better! Thanks.

Contributor

paveljanik commented Feb 10, 2016

Works much better! Thanks.

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Feb 10, 2016

Member

@paveljanik In Makefile.qt.include:

EXTRA_LIBRARIES += qt/libbitcoinqt.a

So that should take care of it.

Member

theuni commented Feb 10, 2016

@paveljanik In Makefile.qt.include:

EXTRA_LIBRARIES += qt/libbitcoinqt.a

So that should take care of it.

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Feb 10, 2016

Member

Thanks, ut ack.

Member

theuni commented Feb 10, 2016

Thanks, ut ack.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 15, 2016

Member

BTW (no need to be in this PR) it would be nice to add a 'crystal clear' check to the test suite, could be interleaved with the current build process, just taking a snapshot of files here and there, then cleaning afterwards and checking:

./autogen.sh
git ls-files -o > $TMPDIR/after_autogen.txt
./configure
git ls-files -o > $TMPDIR/after_configure.txt
make 
...
make clean
git ls-files -o > $TMPDIR/after_clean.txt
make distclean
git ls-files -o > $TMPDIR/after_distclean.txt
# Check
diff $TMPDIR/after_configure.txt $TMPDIR/after_clean.txt
diff $TMPDIR/after_distclean.txt $TMPDIR/after_autogen.txt

(needs at least one excemption: looks like make clean leaves behind .dirstamp files which do get cleaned up by make distclean)

Member

laanwj commented Feb 15, 2016

BTW (no need to be in this PR) it would be nice to add a 'crystal clear' check to the test suite, could be interleaved with the current build process, just taking a snapshot of files here and there, then cleaning afterwards and checking:

./autogen.sh
git ls-files -o > $TMPDIR/after_autogen.txt
./configure
git ls-files -o > $TMPDIR/after_configure.txt
make 
...
make clean
git ls-files -o > $TMPDIR/after_clean.txt
make distclean
git ls-files -o > $TMPDIR/after_distclean.txt
# Check
diff $TMPDIR/after_configure.txt $TMPDIR/after_clean.txt
diff $TMPDIR/after_distclean.txt $TMPDIR/after_autogen.txt

(needs at least one excemption: looks like make clean leaves behind .dirstamp files which do get cleaned up by make distclean)

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 15, 2016

Member

Tested using this patch, and with the above steps. Only one snag:

  • make clean leaves behind one file that is not present after configure: src/obj/build.h, but this is cleaned up by make distclean, so may be a false positive

ACK, it solves the issue I raise in #7501

Member

laanwj commented Feb 15, 2016

Tested using this patch, and with the above steps. Only one snag:

  • make clean leaves behind one file that is not present after configure: src/obj/build.h, but this is cleaned up by make distclean, so may be a false positive

ACK, it solves the issue I raise in #7501

@laanwj laanwj merged commit ae6eca0 into bitcoin:master Feb 15, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Feb 15, 2016

Merge #7504: Crystal clean make clean
ae6eca0 make clean should clean .a files (Pavel Janík)
@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Feb 15, 2016

Member

@laanwj 'make distcheck' should do what you're after, i think? Been a while since I've tried it, I wouldn't be surprised if it's currently broken.

Member

theuni commented Feb 15, 2016

@laanwj 'make distcheck' should do what you're after, i think? Been a while since I've tried it, I wouldn't be surprised if it's currently broken.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 16, 2016

Member

@theuni Yes, that's probably what I've been emulating. I had no idea there was a built-in command for that.

Member

laanwj commented Feb 16, 2016

@theuni Yes, that's probably what I've been emulating. I had no idea there was a built-in command for that.

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7504: Crystal clean make clean
ae6eca0 make clean should clean .a files (Pavel Janík)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7504: Crystal clean make clean
ae6eca0 make clean should clean .a files (Pavel Janík)

codablock added a commit to codablock/dash that referenced this pull request Dec 9, 2017

Merge #7504: Crystal clean make clean
ae6eca0 make clean should clean .a files (Pavel Janík)

codablock added a commit to codablock/dash that referenced this pull request Dec 9, 2017

Merge #7504: Crystal clean make clean
ae6eca0 make clean should clean .a files (Pavel Janík)

codablock added a commit to codablock/dash that referenced this pull request Dec 11, 2017

Merge #7504: Crystal clean make clean
ae6eca0 make clean should clean .a files (Pavel Janík)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment