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] declare MANPATH if and only if it's not set #1430

Merged
merged 1 commit into from
Mar 19, 2017

Conversation

PeterDaveHello
Copy link
Collaborator

By manpath's man page in Ubuntu 16.04:

If $MANPATH is set, manpath will simply display its contents and issue
a warning.

By fa22d71 for #1413, nvm now will
declare the "MANPATH" variable, no matter if it's set or not, so in the
situation that $MANPATH is set, you'll get the warning:

manpath: warning: $MANPATH set, ignoring /etc/manpath.config

@ljharb
Copy link
Member

ljharb commented Mar 15, 2017

To confirm, did you file this in response to 186509b#commitcomment-21338872?

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was originally made in fa22d71, to fix #1413. Reverting this change reopens #1413.

@PeterDaveHello
Copy link
Collaborator Author

That comment seem to copy my commit message and comment, not sure what should I do there?

@ljharb
Copy link
Member

ljharb commented Mar 17, 2017

Don't worry about that comment :-)

It sounds like we've got two separate issues - the fix for one creates the other. Can we find a way to fix both?

@PeterDaveHello
Copy link
Collaborator Author

PeterDaveHello commented Mar 17, 2017

@ljharb workaround: quite manpage's warning output with -q, what do you think?

@PeterDaveHello
Copy link
Collaborator Author

Isn't this patch fixed both? I though it fixed both.

@ljharb
Copy link
Member

ljharb commented Mar 17, 2017

Oh, hmm. It might indeed; I thought it was a simple revert

Let me test it locally to make sure.

By manpath's man page in Ubuntu 16.04:

> If $MANPATH is set, manpath will simply display its contents and issue
> a warning.

By fa22d71 for nvm-sh#1413, `nvm` now will
declare the "MANPATH" variable, no matter if it's set or not, so in the
situation that $MANPATH is set, you'll get the warning:

> manpath: warning: $MANPATH set, ignoring /etc/manpath.config
@ljharb ljharb merged commit b879628 into nvm-sh:master Mar 19, 2017
@PeterDaveHello PeterDaveHello deleted the manpath-fix branch March 19, 2017 08:42
@ljharb
Copy link
Member

ljharb commented Mar 20, 2017

@PeterDaveHello hmm, every master build since this was merged is erroring on 3 or 4 of the 4 "slow" tests, repeatedly. Any idea? https://travis-ci.org/creationix/nvm/builds/212626859

@PeterDaveHello
Copy link
Collaborator Author

I'm not sure if it's related, unless b879628 passed here: https://travis-ci.org/creationix/nvm/builds/212504980

@PeterDaveHello
Copy link
Collaborator Author

@ljharb it looks no problem by my side, confused 😕
https://travis-ci.org/PeterDaveHelloKitchen/nvm/builds/213150903

@PeterDaveHello
Copy link
Collaborator Author

Maybe it's Travis CI's problem, also be fine in this repo now:
https://travis-ci.org/creationix/nvm/builds/213148436

@ljharb
Copy link
Member

ljharb commented Mar 21, 2017

I had to rerun the builds many times before they would pass.

I agree it doesn't seem related, but this PR is the first commit on master where the problem occurred.

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

2 participants