move res/ to src/dmd/res by WalterBright · Pull Request #2821 · dlang/dlang.org

WalterBright · 2020-06-15T04:53:56Z

dlang-bot · 2020-06-15T04:53:58Z

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Geod24 · 2020-06-15T04:55:50Z

That won't work, because the file doesn't exists yet. Either conditionally check for it (the most resilient approach) or copy it in DMD before merging this PR.

WalterBright · 2020-06-15T08:17:49Z

@CyberShadow in CyberShadow/DAutoTest:

0.8.4/vibe-d/textfilter -I/home/dtest/DAutoTest/work/home/.dub/packages/vibe-d-0.8.4/vibe-d/tls -I/home/dtest/DAutoTest/work/home/.dub/packages/openssl-1.1.6_1.0.1g/openssl -Jdpl-docs/views -J../dmd/res -J../dmd/generated/dub -J/home/dtest/DAutoTest/work/home/.dub/packages/ddox-0.16.11/ddox/views -J/home/dtest/DAutoTest/work/home/.dub/packages/hyphenate-1.1.1/hyphenate/patterns dpl-docs/source/app.d dpl-docs/source/ddox_main.d /home/dtest/DAutoTest/work/home/.dub/packages/vibe-core-1.4.0/vibe-core/source/vibe/appmain.d -vcolumns

Note the wrong -J../dmd/res Where is this coming from? If I grep for textfilter I get no hits anywhere in the dlang.org repository.

WalterBright · 2020-06-15T08:23:28Z

@Geod24 yeah, well, if I leave the file in both places, then I can never escape the trap of needing to pull both PRs for either to work, since I am unable to otherwise find all the places where dmd/res is hardwired into dlang.org.

Geod24 · 2020-06-15T08:29:08Z

@Geod24 yeah, well, if I leave the file in both places, then I can never escape the trap of needing to pull both PRs for either to work, since I am unable to otherwise find all the places where dmd/res is hardwired into dlang.org.

Well no you just need to copy it, pull, then have a pull to remove it, and once it goes green you know you're done ?

wilzbach · 2020-06-15T08:34:14Z

If I grep for textfilter I get no hits anywhere in the dlang.org repository.

Of course, it's a dub sub package of Vibe.d.

since I am unable to otherwise find all the places where dmd/res is hardwired into dlang.org.

As you have found this is the only hard-wired for the ddox build:

https://github.com/dlang/dlang.org/blob/master/posix.mak#L741
https://github.com/dlang/dlang.org/blob/master/posix.mak#L755

Also, this won't fully work yet as for the ddoc the preprocessor uses DMD as library (via DUB):
One example:

https://github.com/dlang/dlang.org/blob/master/ddoc/dub.sdl

You will need to duplicate the file first before being able to remove all references or at least update the dub.sdl at dmd.

edit: the build currently fails because of DMD doesn't support multiple arguments with either semi-colon or comma separation. See dlang/dmd#7863 for details.

wilzbach · 2020-06-15T08:37:10Z

posix.mak

 	find ${PHOBOS_LATEST_DIR}/etc ${PHOBOS_LATEST_DIR}/std -name '*.d' | \
 		sed -e /unittest.d/d | sort >> $G/.latest-files.txt
-	${DMD_LATEST} -J$(DMD_LATEST_DIR)/res -J$(dir $(DMD_LATEST)) -c -o- -version=CoreDdoc \
+	${DMD_LATEST} -J$(DMD_LATEST_DIR)/src/dmd/res;$(DMD_LATEST_DIR)/res -J$(dir $(DMD_LATEST)) -c -o- -version=CoreDdoc \


DMD doesn't support multiple options with comma or semicolon. You rejected this a while ago: dlang/dmd#7863

Are you sure? I looked at the source code, and there's code to do it (buildPath() in mars.d which calls FileName.splitPath()). I tried an example, and it works.

Well, I still think that it's still the reason why the CI fails.
Why not just do:

Suggested change

${DMD_LATEST} -J$(DMD_LATEST_DIR)/src/dmd/res;$(DMD_LATEST_DIR)/res -J$(dir $(DMD_LATEST)) -c -o- -version=CoreDdoc \

${DMD_LATEST} -J$(DMD_LATEST_DIR)/src/dmd/res -J$(DMD_LATEST_DIR)/res -J$(dir $(DMD_LATEST)) -c -o- -version=CoreDdoc \

CyberShadow · 2020-06-15T08:38:20Z

Note the wrong -J../dmd/res Where is this coming from? If I grep for textfilter I get no hits anywhere in the dlang.org repository.

I think you need to updae stringImportPaths in dub.sdl.

WalterBright · 2020-06-15T08:45:15Z

I think you need to updae stringImportPaths in dub.sdl.

If you mean dmd/dub.sdl, it's already done in dlang/dmd#11269

Egg, meet chicken. Chicken, meet egg!

WalterBright · 2020-06-15T08:49:07Z

Well no you just need to copy it, pull, then have a pull to remove it, and once it goes green you know you're done ?

So I put it in both places, it goes green. I remove it from dmd first, now dlang fails because it's dependent on the old. But if I fix dlang first, it doesn't work because it isn't in the right place.

CyberShadow · 2020-06-15T08:50:31Z

Yes, you need to do it in 3 steps: 1. Add it in the new place 2. Change the other repository to use the new place, and 3. Remove it from the old place.

Geod24 · 2020-06-15T08:51:22Z

But if I fix dlang first, it doesn't work because it isn't in the right place.

Why wouldn't it be in the right place, if you duplicated it in the first place ?
Although, come to think of it, the solution is much simpler: Just provide both import path here and DMD should pick the right one without needing the duplication in DMD.

WalterBright · 2020-06-15T08:52:07Z

Just provide both import path here and DMD should pick the right one without needing the duplication in DMD.

Tried that. Doesn't seem to work, either.

Geod24 · 2020-06-15T08:53:45Z

Using two separate -J arguments or one ? Asking because of this comment.

WalterBright · 2020-06-15T08:55:56Z

Yes, you need to do it in 3 steps: 1. Add it in the new place 2. Change the other repository to use the new place, and 3. Remove it from the old place.

The difficulty with that is what I'm having now - I can't find all the places of "use the new place". Part of the reason for that is un-informative log files. By following your suggestion, I do a loop of PRs of removing, failing, adding it back in, fixing, removing, failing, adding it back in, etc. This loop could take days at this rate.

WalterBright · 2020-06-15T08:57:00Z

Using two separate -J arguments or one ?

I tried it with one. It works when I try it, and the code is there in mars.d to spit the paths.

Geod24 · 2020-06-15T08:57:23Z

There's no loop. As long as the removal PR doesn't pass the CI, you know there are places left referencing the file.

WalterBright · 2020-06-15T08:59:22Z

There's no loop. As long as the removal PR doesn't pass the CI, you know there are places left referencing the file.

But I cannot test the dmd PR against the dlang PR until it is pulled! Hence the loop.

CyberShadow · 2020-06-15T09:00:28Z

The difficulty with that is what I'm having now - I can't find all the places of "use the new place". Part of the reason for that is un-informative log files. By following your suggestion, I do a loop of PRs of removing, failing, adding it back in, fixing, removing, failing, adding it back in, etc. This loop could take days at this rate.

You would need to do this (finding all the places) anyway, and there is no general solution to this problem other than migrating to a monorepo or investing in CIs which understand inter-PR dependencies (the CI I was working in 2016 to replace DAutoTest had this).

WalterBright · 2020-06-15T09:01:00Z

Every missed hardcoded reference to res in dlang.org will cost another round of pulling and unpulling PRs.

wilzbach · 2020-06-15T09:01:58Z

But I cannot test the dmd PR against the dlang PR until it is pulled! Hence the loop.

Merge this: Copy res/ to src/dmd/res dmd#11278
Merge this dlang.org PR
Merge your dmd PR (which should include a removal of the res)

WalterBright · 2020-06-15T09:02:16Z

You would need to do this (finding all the places) anyway, and there is no general solution to this problem other than migrating to a monorepo or investing in CIs which understand inter-PR dependencies (the CI I was working in 2016 to replace DAutoTest had this).

Thanks for finally understanding what I'm saying :-)

The other solution is to simply pull the dmd one, and then keep fixing the dlang.org one until it passes.

wilzbach · 2020-06-15T09:04:01Z

Every missed hardcoded reference to res in dlang.org will cost another round of pulling and unpulling PRs.

No, you'll duplicate at the beginning and there's no pulling/unpulling. Only additional PRs to dlang.org if you really missed anything. I am happy to handle those PRs if this makes this discussion stop and let's us move to sth. more meaningful.

The other solution is to simply pull the dmd one, and then keep fixing the dlang.org one until it passes.

No, master must stay buildable.

WalterBright · 2020-06-15T09:06:06Z

@wilzbach

merging the dmd PR broke dlang.org because the dlang.org didn't find all the places
revert the dmd PR
merge another dlang.org PR to fix the missed spot
re-merge the dmd PR
goto step 4

Geod24 · 2020-06-15T09:06:19Z

But I cannot test the dmd PR against the dlang PR until it is pulled! Hence the loop.

Three contributors have given you a way to work through it, but you keep ignoring it.
We not only gave you one, but two ways to deal with the problem.

You keep insisting that "-Jfoo1;foo2" works. It does not.

% cat walter.d
pragma(msg, import("resource"));
% ll folder1
% ll folder2
total 8
-rw-r--r--  1 geod24  staff  7 Jun 15 17:57 resource
% cat folder2/resource
Foobar
% dmd -o- "-Jfolder2;folder1" walter.d
walter.d(1): Error: file "resource" cannot be found or not in a path specified with -J
walter.d(1):        while evaluating pragma(msg, import("resource"))
% dmd -o- "-Jfolder1;folder2" walter.d
walter.d(1): Error: file "resource" cannot be found or not in a path specified with -J
walter.d(1):        while evaluating pragma(msg, import("resource"))
% dmd -o- -Jfolder1 -Jfolder2 walter.d
Foobar

This is becoming extremely frustrating.

CyberShadow · 2020-06-15T09:07:58Z

Thanks for finally understanding what I'm saying :-)

To clarify, the general problem is when there is no simple way to break up inter-dependent PRs into three steps such as described above. In this case I also don't see the problem with copying the file as the first step (dlang/dmd#11278).

WalterBright · 2020-06-15T09:12:33Z

@Geod24 I tried an example, and it works. The code in mars.d is also there to split the paths. I have no idea why it doesn't work for you.

This is becoming extremely frustrating.

For both of us. I thought that moving the file would be simple.

Geod24 · 2020-06-15T09:13:23Z

@Geod24 I tried an example, and it works.

Can you show your example ?

WalterBright · 2020-06-15T09:17:16Z

Sure:

C:\cbx\mars\bug>..\dmd -c test -Ja;b
DMD v2.077.0-beta.2
 DEBUG
abc


C:\cbx\mars\bug>..\dmd -c test -Jb;a
DMD v2.077.0-beta.2
 DEBUG
abc


C:\cbx\mars\bug>type test.d
    pragma(msg, import("foo.txt"));


C:\cbx\mars\bug>dir b
 Volume in drive C has no label.
 Volume Serial Number is 6E3B-6D44

 Directory of C:\cbx\mars\bug\b

06/15/2020  01:39 AM                 5 foo.txt
06/15/2020  02:09 AM    <DIR>          ..
06/15/2020  02:09 AM    <DIR>          .
               1 File(s)              5 bytes
               2 Dir(s)  282,400,731,136 bytes free

C:\cbx\mars\bug>dir a
 Volume in drive C has no label.
 Volume Serial Number is 6E3B-6D44

 Directory of C:\cbx\mars\bug\a

06/15/2020  02:09 AM    <DIR>          ..
06/15/2020  02:09 AM    <DIR>          .
               0 File(s)              0 bytes
               2 Dir(s)  282,400,731,136 bytes free

C:\cbx\mars\bug>dir foo.txt
 Volume in drive C has no label.
 Volume Serial Number is 6E3B-6D44

 Directory of C:\cbx\mars\bug

File Not Found

CyberShadow · 2020-06-15T09:18:43Z

Semicolon splitting for -J also doesn't work for me (on Linux). Maybe it only works on Windows?

$ echo 'pragma(msg, import("test.txt"));' > test.d
$ mkdir a b
$ echo foo > b/test.txt
$ dmd -o- '-Ja;b' test.d 
test.d(1): Error: file "test.txt" cannot be found or not in a path specified with -J
test.d(1):        while evaluating pragma(msg, import("test.txt"))
$ dmd -o- '-Jb;a' test.d
test.d(1): Error: file "test.txt" cannot be found or not in a path specified with -J
test.d(1):        while evaluating pragma(msg, import("test.txt"))
$ dmd -o- '-Jb' test.d  
foo

Edit: works on Wine. Yep, looks like it's Windows-only for some reason.

WalterBright · 2020-06-15T09:22:35Z

Looking at FileName.splitPath():

                       version (OSX)
                        {
                        case ',':
                        }
                        version (Windows)
                        {
                        case ';':
                        }
                        version (Posix)
                        {
                        case ':':
                        }

@CyberShadow wins that round!

wilzbach · 2020-06-15T09:23:22Z

@WalterBright I merged the copy res PR and update your PR here to only use the newer res import path. When this passes, you should be able to continue with your previous PR.
You can either remove the original res/ directly in your PR or move it into a different one, but that should be the only remaining thing to do.

WalterBright · 2020-06-15T09:25:29Z

@wilzbach thank you. But that'll only work if this PR actually finds all the places. If it doesn't, we'll be looping!

wilzbach · 2020-06-15T09:39:00Z

@wilzbach thank you. But that'll only work if this PR actually finds all the places. If it doesn't, we'll be looping!

There's no looping. If we really missed something, it will show up in a CI of dmd on your PR. Then it will just be a single PR to fix it (as both files still exists for all other repos). No need to "loop" with pulling/unpulling.

CyberShadow · 2020-06-15T09:50:02Z

Then it will just be a single PR to fix it

Hypothetically multiple dlang.org PRs may be needed, as the CI on the DMD repo will only identify the first problem. But there is no need to unmerge.

WalterBright · 2020-06-15T10:03:16Z

Now it's failing with:

.generated/dmd-2.092.1/src/dmd/lambdacomp.d(470): Error: function `dmd.lambdacomp.SerializeVisitor.visit` does not override any function

which doesn't seem to be related to this PR.

wilzbach · 2020-06-15T10:11:47Z

Now it's failing with:
.generated/dmd-2.092.1/src/dmd/lambdacomp.d(470): Error: function `dmd.lambdacomp.SerializeVisitor.visit` does not override any function
which doesn't seem to be related to this PR.

I guess the real error is above:

.generated/dmd-2.092.1/generated/linux/release/64/dmd -J.generated/dmd-2.092.1/src/dmd/res -J.generated/dmd-2.092.1/generated/linux/release/64/ -c -o- -version=CoreDdoc \
	-version=MARS -version=CoreDdoc -version=StdDdoc -Df.generated/.latest-dummy.html \
	-Xf.generated/docs-latest.json -I.generated/phobos-2.092.1 @.generated/.latest-files.txt
.generated/dmd-2.092.1/src/dmd/doc.d(368): Error: file `"default_ddoc_theme.ddoc"` cannot be found or not in a path specified with `-J`
.generated/dmd-2.092.1/src/dmd/visitor.d(120): Error: function `dmd.visitor.SemanticTimeTransitiveVisitor.visit` conflicts with alias `dmd.visitor.SemanticTimeTransitiveVisitor.visit` at .generated/dmd-2.092.1/src/dmd/visitor.d(118)
.generated/dmd-2.092.1/src/dmd/lambdacomp.d(132): Error: function `dmd.lambdacomp.SerializeVisitor.visit` does not override any function
.generated/dmd-2.092.1/src/dmd/lambdacomp.d(176): Error: function `dmd.lambdacomp.SerializeVisitor.visit` does not override any function
.generated/dmd-2.092.1/src/dmd/lambdacomp.d(230): Error: function `dmd.lambdacomp.SerializeVisitor.visit` does not override any function
.generated/dmd-2.092.1/src/dmd/lambdacomp.d(277): Error: function `dmd.lambdacomp.SerializeVisitor.visit` does not override any function
.generated/dmd-2.092.1/src/dmd/lambdacomp.d(293): Error: function `dmd.lambdacomp.SerializeVisitor.visit` does not override any function

That's because we build stable here as well (-> ba9175c).

I'm not sure why the errors below are happening.

WalterBright added the Blocking other work label Jun 15, 2020

WalterBright requested review from CyberShadow, MartinNowak, andralex and wilzbach as code owners June 15, 2020 04:53

WalterBright mentioned this pull request Jun 15, 2020

move res/ subdirectory to src/dmd/res/ dlang/dmd#11269

Merged

WalterBright force-pushed the mv-res branch 2 times, most recently from 0d6e67d to 385b39f Compare June 15, 2020 05:55

wilzbach reviewed Jun 15, 2020

View reviewed changes

move res/ to src/dmd/res

2fe8e39

wilzbach force-pushed the mv-res branch from 385b39f to 2fe8e39 Compare June 15, 2020 09:21

Add both res import paths for ddox stable

ba9175c

wilzbach approved these changes Jun 15, 2020

View reviewed changes

wilzbach merged commit b5ba4bb into dlang:master Jun 15, 2020

wilzbach mentioned this pull request Jun 15, 2020

[2.093] Remove duplicate res path for the stable ddox build #2822

Merged

	${DMD_LATEST} -J$(DMD_LATEST_DIR)/src/dmd/res;$(DMD_LATEST_DIR)/res -J$(dir $(DMD_LATEST)) -c -o- -version=CoreDdoc \
	${DMD_LATEST} -J$(DMD_LATEST_DIR)/src/dmd/res -J$(DMD_LATEST_DIR)/res -J$(dir $(DMD_LATEST)) -c -o- -version=CoreDdoc \

Uh oh!

Comments

Conversation

WalterBright commented Jun 15, 2020

Uh oh!

dlang-bot commented Jun 15, 2020

Bugzilla references

Uh oh!

Geod24 commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

Geod24 commented Jun 15, 2020

Uh oh!

wilzbach commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

WalterBright Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

wilzbach Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

CyberShadow commented Jun 15, 2020

Uh oh!

Geod24 commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

Geod24 commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

Geod24 commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

CyberShadow commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

wilzbach commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

wilzbach commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

Geod24 commented Jun 15, 2020

Uh oh!

CyberShadow commented Jun 15, 2020

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

Geod24 commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

CyberShadow commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WalterBright commented Jun 15, 2020

Uh oh!

wilzbach commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

wilzbach commented Jun 15, 2020 •

edited

Loading

wilzbach Jun 15, 2020 •

edited

Loading

Geod24 commented Jun 15, 2020 •

edited

Loading

CyberShadow commented Jun 15, 2020 •

edited

Loading

wilzbach commented Jun 15, 2020 •

edited

Loading