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

. (... | psub) broken since 3.1.0 #6613

Closed
fairysword opened this issue Feb 16, 2020 · 17 comments
Closed

. (... | psub) broken since 3.1.0 #6613

fairysword opened this issue Feb 16, 2020 · 17 comments
Labels
bug regression
Milestone

Comments

@fairysword
Copy link

fairysword commented Feb 16, 2020

After upgraded to 3.1

source: Error encountered while sourcing file '/var/folders/pj/jn249gcn7ddfrjzj2_9mxjhw0000gp/T//.psub.f0iJSWRByB':
source: No such file or directory

How to fix the error?

@faho
Copy link
Member

faho commented Feb 16, 2020

What's happening here is that psub, to do its work creates a temporary file or directory. And to do that it uses $TMPDIR if it is set or /tmp otherwise.

So it seems that something in your configuration is setting $TMPDIR to a broken value. Find that and fix it.

@faho faho added the question label Feb 16, 2020
@floam
Copy link
Member

floam commented Feb 16, 2020

It looks like a macOS mktemp/TMPDIR path, it sure would be strange that it would be set by the OS to a directory that does not exist. What is the directory in the output of mktemp?

@floam
Copy link
Member

floam commented Feb 16, 2020

@faho I kind of doubt this is as simple as a bad configuration setting TMPDIR wrong - people on Macs don't really set it, launchd does. Someone else reported the same thing just two days ago: #6594 (comment)

But his explanation that it was related to . vs source makes no sense.

@faho
Copy link
Member

faho commented Feb 16, 2020

Yeah, they were doing

 . (rbenv init -|psub)

which is about two layers too convoluted - just do rbenv init - | source

Now why we're erroring here with . I'm not sure. Do we delete the tmpfile before that executes or something?

@floam
Copy link
Member

floam commented Feb 16, 2020

Also weird that he was only seeing it with fish -l and not fish. Can't figure out what's up with that either. I wonder if he rebooted or something in between these reported changes which could recreate the directories if something had happened to them.

@faho
Copy link
Member

faho commented Feb 16, 2020

A simple test with . (command echo echo wurst | psub) works for me.

Also weird that he was only seeing it with fish -l and not fish

Easy explanation: Doing rbenv only in login shells.

@floam
Copy link
Member

floam commented Feb 16, 2020

That's simple: Doing rbenv only in login shells.

Oh, right.

romanlevin added a commit to romanlevin/rbenv that referenced this issue Feb 16, 2020
Since fish 3.10 at least, the current way of loading rbenv in `fish` is misbehaving, at least in some environments:

```
source: Error encountered while sourcing file '/var/folders/pj/jn249gcn7ddfrjzj2_9mxjhw0000gp/T//.psub.f0iJSWRByB':
source: No such file or directory
```

This changes the initialization to the method recommended by a `fish` developer here: fish-shell/fish-shell#6613 (comment)
@faho
Copy link
Member

faho commented Feb 17, 2020

Okay, I can't reproduce this. I literally put

status --is-interactive; and source (rbenv init - | psub)

into my config.fish and it just works?

So it might be mac-specific, and I don't have a mac. So I'm going to need one of our many mac-using users and developers to figure this one out. I'll be happy to answer any questions I can.

@zanchey
Copy link
Member

zanchey commented Feb 17, 2020

. (command echo echo wurst | psub) fails on my macOS machine. I'll dig into it.

@zanchey
Copy link
Member

zanchey commented Feb 17, 2020

I think what happens is that the outer job completes, which leads to the cleanup function running and deleting the file before source actually has a chance to read it. Might need @ridiculousfish to take a look.

@zanchey zanchey added bug and removed question labels Feb 17, 2020
@zanchey zanchey added this to the fish 3.1.1 milestone Feb 17, 2020
@zanchey zanchey changed the title Error encountered while sourcing file after upgraded to 3.1 source (... | psub) broken since 3.1.0 Feb 17, 2020
@floam floam changed the title source (... | psub) broken since 3.1.0 . (... | psub) broken since 3.1.0 Feb 17, 2020
@faho
Copy link
Member

faho commented Feb 17, 2020

@zanchey Can you confirm it works with source instead of .?

@zanchey
Copy link
Member

zanchey commented Feb 17, 2020

Indeed, it does.

@zanchey
Copy link
Member

zanchey commented Feb 17, 2020

It works in master (65883e0), but not in 3.1.0. Bisects to 6bf9ae9 as first "fixed" commit, but I'm not sure if that's safe to cherry-pick.

@faho
Copy link
Member

faho commented Feb 17, 2020

Doesn't apply cleanly. In particular it pulls in the process_type_t stuff.

Tbh I'd rather just have a simplified version of it.

romanlevin added a commit to romanlevin/rbenv that referenced this issue Feb 17, 2020
Since fish 3.10 at least, the current way of loading rbenv in `fish` is misbehaving, at least in some environments:

```
source: Error encountered while sourcing file '/var/folders/pj/jn249gcn7ddfrjzj2_9mxjhw0000gp/T//.psub.f0iJSWRByB':
source: No such file or directory
```

This changes the initialization to the method recommended by a `fish` developer here: fish-shell/fish-shell#6613 (comment)
phette23 added a commit to phette23/fishrc that referenced this issue Feb 18, 2020
see fish-shell/fish-shell#6613 using the new recommended
init snippet works
@nickolasclarke
Copy link

nickolasclarke commented Feb 19, 2020

I am also seeing this . vs source issue with my pyenv configuration. The following breaks:

set -x PATH "/home/nclarke/.pyenv/bin" $PATH
status --is-interactive; and . (pyenv init -|psub)
status --is-interactive; and . (pyenv virtualenv-init -|psub)

with output:

source: Error encountered while sourcing file “/tmp/.psub.BJGzc7yHU6”:
source: No such file or directory
source: Error encountered while sourcing file “/tmp/.psub.b0QHyQUAZK”:
source: No such file or directory

while the following works fine.

set -x PATH "/home/nclarke/.pyenv/bin" $PATH
status --is-interactive; and source (pyenv init -|psub)
status --is-interactive; and source (pyenv virtualenv-init -|psub)

@faho
Copy link
Member

faho commented Feb 19, 2020

@nickolasclarke You want just:

set -x PATH "/home/nclarke/.pyenv/bin" $PATH
status --is-interactive; and pyenv init - | source
status --is-interactive; and pyenv virtualenv-init - | source

psub is entirely useless here.

(Of course it should still work, and we've fixed it in master. We're just figuring out the correct way to fix it for 3.1.1)

@nickolasclarke
Copy link

nickolasclarke commented Feb 19, 2020

@faho noted, thanks. I'm tracking down which script automatically added these lines and will open a PR.

nickolasclarke added a commit to nickolasclarke/pengwin-setup that referenced this issue Feb 19, 2020
This addresses a [regression](fish-shell/fish-shell#6613) introduced in fish 3.1 and aligns with best practices for fish shell scripting.
faho pushed a commit to faho/fish-shell that referenced this issue Feb 20, 2020
The `function --on-job-exit caller` feature allows a command substitution
to observe when the parent job exits. This has never worked very well - in
particular it is based on job IDs, so a function that observes this will
run multiple times. Implement it properly.

Do this by having a not-recycled "internal job id".

This is only used by psub, but ensure it works properly none-the-less.

faho:
Backport of 6bf9ae9

Fixes fish-shell#6613
faho pushed a commit that referenced this issue Feb 20, 2020
The `function --on-job-exit caller` feature allows a command substitution
to observe when the parent job exits. This has never worked very well - in
particular it is based on job IDs, so a function that observes this will
run multiple times. Implement it properly.

Do this by having a not-recycled "internal job id".

This is only used by psub, but ensure it works properly none-the-less.

faho:
Backport of 6bf9ae9

Fixes #6613
@faho faho closed this as completed Feb 20, 2020
jacegu added a commit to jacegu/jump that referenced this issue Mar 18, 2020
Updated on README.md in 8995bd8 but
still had the old snippet in man pages. The old snippet was causing an
alert with fish 3.1.0 like the one described in:
fish-shell/fish-shell#6613
jacegu added a commit to jacegu/jump that referenced this issue Mar 18, 2020
Updated on README.md in 8995bd8 but
still had the old snippet in man pages. The old snippet was causing an
alert with fish 3.1.0 like the one described in:
fish-shell/fish-shell#6613
NJichev added a commit to NJichev/jump that referenced this issue Apr 26, 2020
Using `.` fails to create a tmp file and source it.
Said issue described [here](fish-shell/fish-shell#6613)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug regression
Projects
None yet
Development

No branches or pull requests

5 participants