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 flux path detection when install path contains symlinks #1122

Merged
merged 1 commit into from Jul 24, 2017

Conversation

Projects
None yet
4 participants
@grondo
Copy link
Contributor

grondo commented Jul 24, 2017

The check for whether the flux command driver is installed or in
the build tree does not canonicalize X_BINDIR, so if this path
includes a symlink the test will incorrectly fail.

Canonicalize the path using realpath(3) before the comparison
to avoid missing detection of installed flux. If realpath() returns
ENOENT, then the install path doesn't exist and we must be running
from the builddir.

Fixes #1121

cmd/flux.c: canonicalize bindir in flux_is_installed
The check for whether the flux command driver is installed or in
the build tree does not canonicalize X_BINDIR, so if this path
includes a symlink the test will incorrectly fail.

Canonicalize the path using `realpath(3)` before the comparison
to avoid missing detection of installed flux. If realpath() returns
ENOENT, then the install path doesn't exist and we must be running
from the builddir.

Fixes #1121
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 24, 2017

Coverage Status

Coverage decreased (-0.05%) to 78.196% when pulling c9836ea on grondo:issue#1121 into 1b8d54b on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 24, 2017

Codecov Report

Merging #1122 into master will decrease coverage by 0.05%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #1122      +/-   ##
==========================================
- Coverage   77.98%   77.92%   -0.06%     
==========================================
  Files         157      157              
  Lines       26035    26039       +4     
==========================================
- Hits        20303    20292      -11     
- Misses       5732     5747      +15
Impacted Files Coverage Δ
src/cmd/flux.c 84.26% <80%> (-0.22%) ⬇️
src/common/libflux/info.c 76.47% <0%> (-2.95%) ⬇️
src/common/libflux/request.c 89.04% <0%> (-1.37%) ⬇️
src/common/libflux/mrpc.c 85.31% <0%> (-1.2%) ⬇️
src/common/libflux/response.c 83.76% <0%> (-0.86%) ⬇️
src/common/libflux/message.c 81.33% <0%> (-0.72%) ⬇️
src/broker/module.c 82.25% <0%> (-0.29%) ⬇️
src/modules/connector-local/local.c 70.06% <0%> (-0.21%) ⬇️
src/common/libflux/handle.c 83.67% <0%> (ø) ⬆️
src/cmd/flux-event.c 66.66% <0%> (+1.11%) ⬆️
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Jul 24, 2017

Thanks!

@garlick garlick merged commit ccf25dc into flux-framework:master Jul 24, 2017

4 checks passed

codecov/patch 80% of diff hit (target 77.98%)
Details
codecov/project Absolute coverage decreased by -0.05% but relative coverage increased by +2.01% compared to 1b8d54b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.05%) to 78.196%
Details

@grondo grondo referenced this pull request Aug 23, 2017

Closed

0.8.0 Release #1160

@grondo grondo deleted the grondo:issue#1121 branch Apr 26, 2018

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.