rdmd fixes (issues 14296, 16962 and 16978)#207
Conversation
…No such file or directory The actual fix was done by the revert. This simply adds a test.
The actual fix was done by the revert. This simply adds a test.
|
@wilzbach CI scripts should use fixed versions of components from other repositories (that are not being tested). Digger was "broken" (it failed to build due to a warning in ae) which was causing tests to fail. I've now pushed a fix, but Dub can't pick it up:
This is despite the tag existing on GitHub, of course. I think this is an instance of dlang/dub#1000, i.e. Dub being "broken by design" for now as far as deployment goes, so it's not usable for CI or deployment. |
|
Oh, OK. Tests are failing because #202 isn't in stable. |
|
Revert of #194 is fine with me. As I said in the discussion for issue 16962, I don't really have time to look into this at the moment. |
| ~ [ "-of"~fullExeTemp ] | ||
| ~ [ "-od"~objDir ] | ||
| ~ [ "-I"~dirName(root) ] | ||
| ~ [ root ]; |
rdmd.d
Outdated
| // regardless of the operating system it's running on. | ||
| std.file.write(rspName, array(map!escapeWindowsArgument(todo)).join(" ")); | ||
|
|
||
| todo = [ "@"~rspName ]; |
| todo = [ "@"~rspName ]; | ||
| } | ||
|
|
||
| immutable result = run([ compiler ] ~ todo); |
rdmd.d
Outdated
| auto depsGetter = | ||
| // "cd "~shellQuote(rootDir)~" && " | ||
| [ compiler ] ~ compilerFlags ~ | ||
| ["-v", "-o-", rootModule, "-I"~rootDir]; |
| { | ||
| enum objExt = ".o"; | ||
| enum binExt = ""; | ||
| enum libExt = ".a"; |
rdmd_test.d
Outdated
| ~this() | ||
| { | ||
| import core.thread; | ||
| Thread.sleep(100.msecs); // Hack around Windows locking the directory |
There was a problem hiding this comment.
Put a version around it? Are there better workarounds? We shouldn't have people wait 100 extra milliseconds on all platforms. Note that currently "rdmd hello.d" takes 200ms on a typical Posix system to build.
I'll ask for changes because of this, rest are optional nits.
There was a problem hiding this comment.
I didn't write this code (it was a refactoring), but might as well.
rdmd_test.d
Outdated
|
|
||
| version (linux) | ||
| { | ||
| TmpDir srcDir = "rdmdTest"; |
|
Please review commit by commit. You are reviewing changes in revert / refactoring commits. |
|
@CyberShadow well I clicked on "Files changed", wouldn't that comprise everything? |
|
Yes, but that's generally a broken way to review a multi-commit PR (where the successive commits aren't fixups, and they aren't here). |
|
I'll add the requested changes anyhow. |
Oh, and I should point out that that particular hack was in the test suite, not rdmd itself. But yeah, point stands. The problem is usually due to Windows Defender, the standard Windows anti-malware, or some other antivirus locking the directory so it could have a look at the files we create. We've had problems with it in Phobos tests as well. |
|
core.exception.AssertError@rdmd_test.d(139): /tmp/.rdmd-1000/eval.E60124B13F32A50795E744C041B53089.d(6): Error: module cstream is in file 'std/cstream.d' which cannot be read |
|
Yes, the fix for that is on another branch (master) as mentioned above. |
|
oh lemme take a lookie |
|
@CyberShadow @MartinNowak so what's the sequence needed to do this right? Pull #202 in stable first? |
|
@CyberShadow @MartinNowak ping |
|
I think we can cherry-pick #202 here. @MartinNowak ? Or we can ignore the CI status, since stable was failing without or with this PR. |
I think we should, because it was fixing a real problem |
|
Yes, cherry-picking is OK if necessary, but generally all fixes of released issues should always go into stable. |
6 participants
No description provided.