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

fix MANPATH for Ubuntu, and tidy up travis dep builder #877

Merged
merged 3 commits into from Oct 26, 2016

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Oct 26, 2016

No description provided.

@garlick garlick added the review label Oct 26, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 26, 2016

Coverage Status

Coverage increased (+0.007%) to 75.713% when pulling 699bbaa on garlick:misc into 342b127 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 26, 2016

Current coverage is 72.13% (diff: 100%)

Merging #877 into master will increase coverage by 0.08%

@@             master       #877   diff @@
==========================================
  Files           156        156          
  Lines         26942      26947     +5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          19412      19438    +26   
+ Misses         7530       7509    -21   
  Partials          0          0          
Diff Coverage File Path
•••••••••• 100% src/cmd/flux.c

Powered by Codecov. Last update 9cea79b...86bdfb9

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 26, 2016

It would probably be pretty easy to test the new case by unsetting MANPATH and run under flux or flux env...

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 26, 2016

I wonder if we also have to handle set but empty MANPATH? On my simple tests an empty MANPATH and unset MANPATH seem to be handled the same (system MANPATH is used).

For testing, perhaps something simple like this would work?

test_expect_success 'flux appends colon to missing or unset MANPATH' '
      (unset MANPATH && flux /usr/bin/printenv | grep "MANPATH=.*:$") &&
      MANPATH= flux /usr/bin/printenv | grep "MANPATH=.*:$"
'

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Oct 26, 2016

Sounds good! I'll check for empty too and add that test.

@garlick garlick force-pushed the garlick:misc branch from 699bbaa to 86bdfb9 Oct 26, 2016

garlick added some commits Oct 25, 2016

cmd/flux: add trailing colon to MANPATH
On Ubuntu systems, setting MANPATH overrides the system manual
page search path, unless a colon is appended to the MANPATH.
On other systems a trailing colon would have no effect.

If MANPATH is unset or empty, append the flux manpath, then a
colon.  Otherwise, just append the flux manpath.

Fixes #745.
travis-ci: drop bogus zeromq options
As found by @SteVwonder, the configure options for zeromq were
not being used.  It turns out we don't want them anyway so delete.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 26, 2016

Coverage Status

Coverage increased (+0.09%) to 75.748% when pulling 86bdfb9 on garlick:misc into 9cea79b on flux-framework:master.

@grondo grondo merged commit 11bb1a2 into flux-framework:master Oct 26, 2016

4 checks passed

codecov/patch 100% of diff hit (target 72.05%)
Details
codecov/project 72.13% (+0.08%) compared to 9cea79b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.09%) to 75.748%
Details
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 26, 2016

Thanks!

@grondo grondo removed the review label Oct 26, 2016

@garlick garlick referenced this pull request Oct 26, 2016

Closed

0.5.0 release notes #879

@garlick garlick deleted the garlick:misc branch Oct 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.