dlang/dub

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

Abscissa opened this issue · 5 comments

$ 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.

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\", ...);.

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 ....

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.

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.

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"?