Skip to content
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

build: Exit on error in autogen.sh plus other cleanup #1163

Merged
merged 1 commit into from
Aug 31, 2017
Merged

build: Exit on error in autogen.sh plus other cleanup #1163

merged 1 commit into from
Aug 31, 2017

Conversation

morrone
Copy link
Contributor

@morrone morrone commented Aug 25, 2017

Make autogen.sh exit after any of the commands fail.

Remove the "-I config" option from the autoreconf command
which appears to be redundant with either AC_CONFIG_AUX_DIR([config])
or AC_CONFIG_MACRO_DIR([config]) in the configure.ac file.

Stop removing autom4te.cache. We can't remember why we did that.

Resolves #1162

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 78.104% when pulling d0a3416 on morrone:autogen_changes into e2b5e8d on flux-framework:master.

@codecov-io
Copy link

codecov-io commented Aug 28, 2017

Codecov Report

Merging #1163 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1163      +/-   ##
==========================================
- Coverage   77.75%   77.75%   -0.01%     
==========================================
  Files         158      158              
  Lines       29265    29266       +1     
==========================================
  Hits        22756    22756              
- Misses       6509     6510       +1
Impacted Files Coverage Δ
src/common/libkvs/kvs_watch.c 86.34% <0%> (-0.89%) ⬇️
src/common/libutil/dirwalk.c 92.25% <0%> (-0.71%) ⬇️
src/broker/overlay.c 71.67% <0%> (-0.35%) ⬇️
src/broker/module.c 83.28% <0%> (-0.28%) ⬇️
src/common/libflux/message.c 81.17% <0%> (-0.24%) ⬇️
src/common/libcompat/handle.c 84.97% <0%> (+0.03%) ⬆️
src/common/libflux/handle.c 83.91% <0%> (+0.24%) ⬆️
src/common/libkvs/kvs.c 64.87% <0%> (+0.25%) ⬆️
src/modules/kvs/kvs.c 62.98% <0%> (+0.25%) ⬆️
src/common/libflux/future.c 89.1% <0%> (+0.49%) ⬆️

@garlick
Copy link
Member

garlick commented Aug 28, 2017

Thanks for the cleanup!

This needs to be rebased. While you're at it, any reason not to just do #!/bin/sh -e instead of adding the individual tests?

Make autogen.sh exit after any of the commands fail.

Remove the "-I config" option from the autoreconf command
which appears to be redundant with either AC_CONFIG_AUX_DIR([config])
or AC_CONFIG_MACRO_DIR([config]) in the configure.ac file.

Stop removing autom4te.cache.  We can't remember why we did that.

Resolves #1162
@morrone
Copy link
Contributor Author

morrone commented Aug 31, 2017

If we use -e, we could also use -x and not need any echo statements. But I don't really care either way.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 78.117% when pulling 2759907 on morrone:autogen_changes into e255d0f on flux-framework:master.

@grondo
Copy link
Contributor

grondo commented Aug 31, 2017

Bonked Travis on the noggin. One build hung after python tests (output stalled so Travis killed the build), so I restarted that build.

@grondo
Copy link
Contributor

grondo commented Aug 31, 2017

If we use -e, we could also use -x and not need any echo statements. But I don't really care either way.

This seems fine to merge now without changes. It fixes the issues in question -- if someone wants to enhance the script to use -x and -e in the future there's not reason not to submit another PR

@grondo grondo merged commit a3ebd15 into flux-framework:master Aug 31, 2017
@morrone morrone deleted the autogen_changes branch September 8, 2017 19:00
@grondo grondo mentioned this pull request May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants