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

0.9.24-beta.2: Infinite new-process recursion using dub decribe within preGenerateCommands #616

Closed
Abscissa opened this issue Jul 3, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@Abscissa
Copy link
Contributor

commented Jul 3, 2015

$ git clone https://github.com/Abscissa/safeArg.git
$ cd safeArg
$ dub describe

Works on DUB 0.9.23. Hangs on DUB 0.9.24-beta.2. Same thing for dub run.

@s-ludwig

This comment has been minimized.

Copy link
Member

commented Jul 5, 2015

For me it hangs in the pre-generate command of "gen-package-version". I have a lot of "bootstrap.exe" processes visible in task manager. The only thing that happens within DUB is a spawnShell("cd /D C:\Users\sludwig\AppData\Roaming\dub\packages\gen-package-version-1.0.2\ && rdmd helper/bootstrap.d C:\Users\sludwig\AppData\Roaming\dub\packages\scriptlike-0.9.1\", ...);.

@Abscissa

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2015

I've dug a little more and it turns out that it's hanging inside a nested invocation of dub. But we didn't notice that because one of my tools is capturing the stdout for that invocation instead of being forwarding it through to the console.

Go into C:\Users\sludwig\AppData\Roaming\dub\packages\gen-package-version-1.0.2\src\genPackageVersion\fetchDubInfo.d. Change line 62 from:

auto rawJson = runCollect("dub describe");

to:

auto rawJson = run("dub describe --vverbose");

Then try again and you'll notice that invocation of dub is hanging right after several lines of iterating dir ....

@Abscissa Abscissa changed the title 0.9.24-beta.2 hangs on specific project 0.9.24-beta.2: Infinite new-process recursion using dub decribe within preGenerateCommands Jul 10, 2015

@Abscissa

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2015

Figured out what's happening:

Due to dub describe's refactoring with the new TargetDescriptionGenerator, dub describe now runs the project's preGenerateCommands (it didn't run them in 0.9.23). My preGenerateCommands indirectly invoke dub describe, so this created an infinite recursion.

On one hand, it does make some sense that dub describe should run the preGenerateCommands, because otherwise the list of sourceFiles (and maybe some others?) could be inaccurate. However, this does create a problem: dub describe can no longer be used directly or indirectly within a project's preGenerateCommands without manually detecting and breaking the inevitable infinite recursion. Not sure what the right solution is.

@s-ludwig

This comment has been minimized.

Copy link
Member

commented Jul 10, 2015

Oh, that makes sense. One trivial possibility, which I actually almost had implemented because I already stumbled over something similar, would be to add a --skip-generate-commands switch for dub describe.

But that IMO wouldn't be completely satisfying, because it's opt-in and thus will still be easy to get wrong. At least it would require an additional warning/error message in case of recursive invocations. But if we have that logic, we can as well replace the proposed switch by that automated logic.

So... any idea for the recursion detection other than creating a temporary .dub.pid file in the package directory for that? It does have the drawback that it wouldn't work in read-only directories, but preGenerateCommands don't make much sense there anyway currently.

PS: Another idea: add a DUB_DESCRIBE=true environment variable and use that instead. That solves the read-only issue and is easier to implement, but wouldn't work if "dub describe" is used across multiple packages - DUB_DESCRIBE=package1,package2,... should work though.

@Abscissa

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2015

One trivial possibility, which I actually almost had implemented because I already stumbled over something similar, would be to add a --skip-generate-commands switch for dub describe.

Actually, I was just about to suggest the reverse: dub describe --run-generate-commands, with the default being to skip them like 0.9.23 does.

But DUB_DESCRIBE=package1,package2,... also sounds good, as long as it's dub itself that detects that envvar and acts accordingly, rather than expecting user scripts to read and handle it.

My only (small) concern with the envvar approach is possible confusion when querying sourceFiles yields different results (fewer files) inside of pre/postGenerateCommands than outside. But that does actually make sense too though, and it'd probably be easier for users than needing to choose "Do I want --run-generate-commands or not"?

@s-ludwig s-ludwig closed this in 7f7bedc Sep 15, 2015

s-ludwig added a commit that referenced this issue Sep 15, 2015

Merge pull request #633 from Abscissa/fix616
Fix #616: Infinite new-process recursion using dub decribe within…
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.