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

kvs: Fix corner case in which symlink points to root dir #1404

Merged
merged 2 commits into from Mar 30, 2018

Conversation

4 participants
@chu11
Copy link
Contributor

chu11 commented Mar 30, 2018

Fix corner case in which a symlink in the kvs points to the root dir, i.e. ".".

I discovered this corner case while working on #1341. I realized users will probably often do:

mysymlink -> namespace/.

i.e. a symlink simply points to the root of another namespace. But root dirs were not handled at all within symlinks.

chu11 added some commits Mar 30, 2018

modules/kvs: Fix lookup corner case on "." symlink
Fix corner case if a symlink is specifically a ".", i.e. the symlink
points to the root directory of the kvs.

@chu11 chu11 added this to To do in multi-user parallel jobs via automation Mar 30, 2018

@chu11 chu11 moved this from To do to In progress in multi-user parallel jobs Mar 30, 2018

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Mar 30, 2018

Oh good one. Yes, I would imagine the "guest KVS namespaces" described in RFC 16 would be linked (instead of "mounted") exactly like that.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 30, 2018

Coverage Status

Coverage increased (+0.001%) to 78.779% when pulling efa543a on chu11:symlinkperiod into a7402b5 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 30, 2018

Codecov Report

Merging #1404 into master will increase coverage by <.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1404      +/-   ##
==========================================
+ Coverage   78.46%   78.47%   +<.01%     
==========================================
  Files         163      163              
  Lines       29981    29983       +2     
==========================================
+ Hits        23526    23528       +2     
  Misses       6455     6455
Impacted Files Coverage Δ
src/modules/kvs/lookup.c 79.78% <66.66%> (+0.08%) ⬆️
src/common/libflux/request.c 88.46% <0%> (-1.29%) ⬇️
src/common/libutil/dirwalk.c 93.57% <0%> (-0.72%) ⬇️
src/common/libkvs/kvs_watch.c 90.98% <0%> (-0.43%) ⬇️
src/cmd/flux-module.c 85.06% <0%> (-0.31%) ⬇️
src/broker/overlay.c 74.45% <0%> (+0.31%) ⬆️
src/common/libutil/blobref.c 98.61% <0%> (+1.38%) ⬆️
src/common/libflux/attr.c 92% <0%> (+2%) ⬆️
@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Mar 30, 2018

code coverage of diff is bad, but its just 1 missed "impossible error" path.

@garlick garlick merged commit 6c5e47d into flux-framework:master Mar 30, 2018

3 of 4 checks passed

codecov/patch 66.66% of diff hit (target 78.46%)
Details
codecov/project 78.47% (+<.01%) compared to a7402b5
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.001%) to 78.779%
Details

multi-user parallel jobs automation moved this from In progress to Done Mar 30, 2018

@grondo grondo referenced this pull request May 10, 2018

Closed

0.9.0 Release #1479

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.