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

set $ENV{SHELL} if it's missing #404

Merged
merged 1 commit into from
Aug 16, 2014
Merged

set $ENV{SHELL} if it's missing #404

merged 1 commit into from
Aug 16, 2014

Conversation

maxhq
Copy link
Contributor

@maxhq maxhq commented Jul 31, 2014

In the rare case when $ENV{SHELL} is not set, perlbrew currently prints out:

/opt/perlbrew/etc/bashrc: line 43: setenv: command not found.
Use of uninitialized value in pattern match (m//) at /loader/0x23ff038/App/perlbrew.pm line 34.

This patch assumes that the parent process is a shell and sets $ENV{SHELL} to the path of it's executable.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) when pulling f6765ea on maxhq:develop into c4782cf on gugod:develop.

@gugod
Copy link
Owner

gugod commented Aug 5, 2014

Hi @maxhq,

The commit f6765ea only works on Linux where /proc is around... It'd be better to make it not broken on Mac and FreeBSD as well.

Also, SHELL is a dependency when interacting with perlbrew via its shell integration, perlbrew itself should be usable as a command. Do you have a init.d script that can demonstrate the error message ?

@maxhq
Copy link
Contributor Author

maxhq commented Aug 15, 2014

Hi gugod,

that's my point: perlbrew should also work without $SHELL being set.

E.g. under Debian if you restart a service with service ... restart it will have a very reduced environment without $SHELL. If this service now starts perlbrew the mentioned error occurs.
Excerpt of /usr/sbin/service on a Debian 6.0.7:
exec env -i LANG="$LANG" PATH="$PATH" TERM="$TERM" "$SERVICEDIR/$SERVICE" ${ACTION} ${OPTIONS}

What about this modification of the patch:
$ENV{SHELL} ||= readlink joinpath("/proc", getppid, "exe") if -d "/proc";

I think the best would be if perlbrew did not use $SHELL at all...

@gugod
Copy link
Owner

gugod commented Aug 15, 2014

@maxhq,

Interesting. Would you mind providing some skeleton of your service script that uses perlbrew ?

The dependency of $SHELL is mainly because of the constrained nature of shell scripting -- to implement use, switch, we inspect $SHELL, then print a dozen of shell script and expect the caller to eval() them. This can be completed rewritten so that the value of $SHELL is passed as a command line parameter.

OTOH, I'd imagine that the service script only need to "read" perlbrew env to bootstrap $PATH or $PERL5LIB -- in those cases, $SHELL should not be required.

gugod added a commit that referenced this pull request Aug 16, 2014
set $ENV{SHELL} if it's missing
@gugod gugod merged commit 00ebd79 into gugod:develop Aug 16, 2014
@gugod
Copy link
Owner

gugod commented Aug 16, 2014

I'll add the if -d "/proc"; check . But we should maybe work on removing the dependency of $SHEL variable.

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Jan 27, 2015
-----------------------------
0.72: # 2014-10-27T21:38:19+0100
- Fix 0.71 breakage on MacOSX when the shell is bash or zsh
- Fix spelling mistake. by @gregoa++

0.71: # 2014-09-13T19:59:15+0200
- Fix a few bugs of having unwanted values in PERL5LIB when switching between libs

0.70: # 2014-09-02T20:41:15+0900
- download the fatpacked perlbrew from an http:// url.
- protect fatpacked perlbrew from PERL5LIB, thanks to dagolden++
- avoid several warnings, thanks to DabeDotCom++
- making perlbrew a bit friendlier to init.d script, thanks to maxhq++ . see also
  gugod/App-perlbrew#404
- make it possible to override PERLBREWURL before invoking installers

0.69: # 2014-06-10T23:32:05+0200
- fix 'perlbrew env' breakage by just avoiding bad local::lib versions.

0.68: # 2014-06-07T15:08:00+0200
- disable SSL cert checks. GH #385.
- "perlbrew download stable" works as on expects. GH #383
- Many fish shell fixes. GH #376 #378 #379
- make 5.21 installable. GH #396
- various documentation fixes.
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.

None yet

3 participants