search

dlang / dub

Package and build management system for D
MIT License
678 stars 227 forks source link

Reduce dependency on the current working directory #2562

Closed WebFreak001 closed 1 year ago

WebFreak001 commented 1 year ago

first commit was needed, because introducing the getWorkingDirectory function call in some module here started actually using the symbols from dub.internal.utils it seems. I'm not sure why it didn't trigger before, but it was an issue for me. I just moved the module destructor that cleans up temporary files into a separate module to work around this.

I changed all path concatenation with getcwd and getWorkingDirectory, all spawnProcess/execute calls and some affected dirEntries calls to use a "toolWorkingDirectory" from GeneratorSettings.

This affects the compiler and the linker, but also minor things such as the ddox tool or dub lint.

This change allows serve-d to always build correctly without needing to chdir the entire application's working directory. (which I can't do, because I have multiple dub instances that may even build in parallel + there were issues that relative paths weren't generated properly)

WebFreak001 commented 1 year ago

ready for review

Geod24 commented 1 year ago

Could you clean up the commit history ? Or should I just squash merge ?

WebFreak001 commented 1 year ago

squash merge should be fine, but I can also do it and keep the cycle fix commit separate.

WebFreak001 commented 1 year ago

rebased into 3 neat commits

rikkimax commented 1 year ago

No bug report, adding a test for #1026 would solve that. Either way looks fine.

WebFreak001 commented 1 year ago

1026 still would need a separate PR, but this PR adds capabilities in code to change the working directory when building.

WebFreak001 commented 1 year ago

@Geod24 can we get this in? I need this for serve-d DUB linting to work properly in multi-workspace and temporary directory cases (like shown in the test)

WebFreak001 commented 1 year ago

@Geod24 @PetarKirov ping, the test here shows what exactly this fixes: https://github.com/dlang/dub/pull/2562/files#diff-9c31e8df9093081ce880575b76f96e115f1778f08c1b4cc51bc9eb111da7b227 (that code didn't work before)

WebFreak001 commented 1 year ago

rebased to target stable now

WebFreak001 commented 1 year ago

not sure what's up with DAutoTest and why it tries to merge into master, but merging into stable seems to work fine. GitHub actions tests also run through successfully now.

WebFreak001 commented 1 year ago

@CyberShadow how can I make the DAutoTest target the stable branch instead of master? (like I'm doing in the PR, which makes the auto tester fail)

CyberShadow commented 1 year ago

Enabled Dub branches to the metarepo builder here: https://github.com/CyberShadow/D-dot-git/commit/06dc0a54eb38719835fb7e07d58497c8c670080d

New metarepo is pushed, should be picked up on the next rebuild, so just rebase.

CyberShadow commented 1 year ago

Um, should this really target stable though? Or is Dub's stable not the same as DMD/Phobos' stable after all?

WebFreak001 commented 1 year ago

we wanted to get this change in for the new release because it's purely an API change outside the DUB CLI and I wanted to use it with serve-d (and, well, it should have been merged before the freeze for the tag already ^^)

See discussion here: https://github.com/dlang/dub/pull/2565

WebFreak001 commented 1 year ago

@Geod24 can I have a review? Tests seem to pass now, just don't know what these required pending tests are that never started.