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

commands: relax run-from-build-tree detection #1515

Merged
merged 2 commits into from May 10, 2018

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented May 10, 2018

Instead of comparing /proc/self/exe to $prefix/bin, compare to ${abs_top_builddir}/src/cmd}.

This may help when running flux under spindle, where the installed flux executable is relocated from its installed path.

The problem is discussed in #1514.

@garlick garlick changed the title flux commands: relax logic for detecting running flux from build tree flux command front end: relax logic for detecting running flux from build tree May 10, 2018

@garlick garlick changed the title flux command front end: relax logic for detecting running flux from build tree commands: relax run-from-build-tree detection May 10, 2018

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented May 10, 2018

restarted one builder after a write error during make distclean:

rm -f Makefile^M
make[3]: Leaving directory `/home/travis/build/flux-framework/flux-core/flux-core-0.8.0-1090-g2187bdc9/_build/sub/doc/man3'^M
make[3]: write error^M
make[2]: *** [distclean-recursive] Error 1^M
make[2]: Leaving directory `/home/travis/build/flux-framework/flux-core/flux-core-0.8.0-1090-g2187bdc9/_build/sub/doc'^M
make[1]: *** [distclean-recursive] Error 1^M
make[1]: Leaving directory `/home/travis/build/flux-framework/flux-core/flux-core-0.8.0-1090-g2187bdc9/_build/sub'^M
make: *** [distcheck] Error 1^M
@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented May 10, 2018

Failed again with

not ok 53 - instance can stop cleanly with subscribers (#1025)

retsarting.

@@ -55,6 +55,7 @@ static struct config default_config[] = {
{ "keydir", NULL, INTREE_KEYDIR },
{ "no_docs_path", INSTALLED_NO_DOCS_PATH, INTREE_NO_DOCS_PATH },
{ "rundir", INSTALLED_RUNDIR, NULL },
{ "bindir", NULL, INTREE_BINDIR },

This comment has been minimized.

@grondo

grondo May 10, 2018

Contributor

One question, not really a review comment: could we also set the INSTALLED_BINDIR here just for consistency? I was initially confused since bindir = ${exec_prefix}/bin is a very common variable used in autoconf.

This comment has been minimized.

@garlick

garlick May 10, 2018

Author Member

Sure, I'll fix that.

@garlick garlick force-pushed the garlick:in_tree_fix branch from f79231e to 04a71e0 May 10, 2018

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented May 10, 2018

Rebased after #1513 and added INSTALLED_BINDIR per comment.

garlick added some commits May 10, 2018

libflux/conf: add bindir attribute
Problem: we need a way to determine the directory
where the flux command front end is built.

Add 'bindir' conf attribute for
   in tree:    ${abs_top_builddir}/src/cmd
   installed:  ${prefix}/bin

Access with flux_conf_get ("bindir", flags).
cmd/flux: reverse logic of in-tree detection
Problem: when flux is launched by spindle, /proc/self/exe
no longer refers to $prefix/bindir and flux attempts
to use paths relative to its build directory to access
the rest of flux.

Instead of comparing /proc/self/exe's symlink target
to $prefix/bindir, compare to ${abs_top_builddir}/src/cmd.
That way the installed executable can be relocated
without harm, and the "special" case is the one where
flux is run out of the build tree, which seems appropriate.

The problem launching flux with spindle is discussed
in issue #1514.
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented May 10, 2018

Thanks!

FWIW, I tried this version with spindle, and we get a bit further, but then hit a strange error in rc1:

 grondo@ipa1:~$ spindle srun --pty -N2 /g/g0/grondo/flux/bin/flux start flux getattr size
2018-05-10T20:37:56.019185Z broker.err[0]: rc1: /g/g0/grondo/flux/etc/flux/rc1: line 26: /g/g0/grondo/flux/etc/flux/rc1.d/01-enclosing-instance: Bad file descriptor
2018-05-10T20:37:56.019531Z broker.err[0]: Run level 1 Exited with non-zero status (rc=1) 0.2s
srun: error: ipa1: task 0: Exited with exit code 1
 grondo@ipa1:~$ srun --pty -N2 /g/g0/grondo/flux/bin/flux start flux getattr size
2

If I comment out the exec of $rcfile in the rc script, a flux session actually runs under spindle:

 grondo@ipa1:~$ spindle srun --pty -N2 /g/g0/grondo/flux/bin/flux start
(flux-FA86Tx) grondo@ipa1:~$ 

Normal commands do work:

(flux-FA86Tx) grondo@ipa1:~$ flux getattr size
2
(flux-FA86Tx) grondo@ipa1:~$ flux kvs ls      
resource

However, commands that are Lua scripts don't:

(flux-FA86Tx) grondo@ipa1:~$ flux exec hostname
lua: cannot open exec: No such file or directory
(flux-FA86Tx) grondo@ipa1:~$ flux wreck ls
lua: cannot open wreck: No such file or directory

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented May 10, 2018

I'll copy results above to the spindle ticket #1514 and merge this once Travis is green since it definitely solves at least one of the spindle issues.

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented May 10, 2018

Restarted gcc builder which failed with a write error.

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented May 10, 2018

Restarted a builder after a write error.

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 10, 2018

Coverage Status

Coverage increased (+0.03%) to 78.887% when pulling 04a71e0 on garlick:in_tree_fix into b531e07 on flux-framework:master.

@grondo grondo merged commit 2ba75df into flux-framework:master May 10, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 78.887%
Details
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented May 10, 2018

Codecov Report

Merging #1515 into master will increase coverage by 0.02%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #1515      +/-   ##
==========================================
+ Coverage   78.56%   78.59%   +0.02%     
==========================================
  Files         165      165              
  Lines       30816    30817       +1     
==========================================
+ Hits        24210    24220      +10     
+ Misses       6606     6597       -9
Impacted Files Coverage Δ
src/common/libflux/conf.c 100% <ø> (ø) ⬆️
src/cmd/flux.c 84.78% <80%> (+0.62%) ⬆️
src/common/libflux/keepalive.c 86.66% <0%> (-6.67%) ⬇️
src/common/libflux/rpc.c 92.37% <0%> (-0.85%) ⬇️
src/common/libutil/base64.c 95.07% <0%> (-0.71%) ⬇️
src/common/libflux/msg_handler.c 87.36% <0%> (-0.37%) ⬇️
src/broker/overlay.c 73.81% <0%> (-0.32%) ⬇️
src/broker/module.c 83.79% <0%> (-0.28%) ⬇️
src/modules/kvs/kvs.c 65.87% <0%> (-0.17%) ⬇️
src/common/libflux/message.c 81.29% <0%> (-0.12%) ⬇️
... and 4 more

@garlick garlick deleted the garlick:in_tree_fix branch Oct 16, 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.