Skip to content

Add DMD to the release pages of Ddoc and Ddox v2#1686

Merged
CyberShadow merged 4 commits intodlang:masterfrom
wilzbach:add-dmd-to-released-pages2
Jun 10, 2017
Merged

Add DMD to the release pages of Ddoc and Ddox v2#1686
CyberShadow merged 4 commits intodlang:masterfrom
wilzbach:add-dmd-to-released-pages2

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Jun 8, 2017

Due to #1682

@wilzbach
Copy link
Contributor Author

wilzbach commented Jun 8, 2017

ddmd/arrayop.d(21): Error: module id is in file 'ddmd/id.d' which cannot be read
import path[0] = ddmd
import path[1] = /home/dtest/DAutoTest/work/tmp/.host_dmd-2.072.2/dmd2/linux/bin64/../../src/phobos
import path[2] = /home/dtest/DAutoTest/work/tmp/.host_dmd-2.072.2/dmd2/linux/bin64/../../src/druntime/import
posix.mak:564: recipe for target '/dev/shm/dtest/work/repo/dlang.org/web/phobos/ddmd_access.html' failed
make[2]: *** [/dev/shm/dtest/work/repo/dlang.org/web/phobos/ddmd_access.html] Error 1

Hmm I really hope we can replace this auto-generated file (dlang/dmd#6837 looks promising).

For now I added id.d to the Makefile targets.

@wilzbach wilzbach force-pushed the add-dmd-to-released-pages2 branch 2 times, most recently from b85bf5e to 31a48ca Compare June 8, 2017 06:35
posix.mak Outdated
$(MAKE) AUTO_BOOTSTRAP=1 --directory=$(DMD_DIR) -f posix.mak src/ddmd/id.d

$(DMD_STABLE_DIR)/src/ddmd/id.d: $(DMD_REL) | $(DMD_STABLE_DIR)
$(MAKE) AUTO_BOOTSTRAP=1 --directory=$(DMD_STABLE_DIR) -f posix.mak src/ddmd/id.d
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently with the changes, DMD isn't by default built from source anymore (or at least the built isn't accessible here).
Anyhow this seems to do the trick.

Copy link
Member

@CyberShadow CyberShadow Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there another solution? Introducing this change here now will cause an inter-dependent deadlock if/when id.d is removed.

Never mind, I see this only applies to the stable snapshot.

@CyberShadow
Copy link
Member

Other 2 are in, rebase please

@CyberShadow
Copy link
Member

Apparently with the changes, DMD isn't by default built from source anymore (or at least the built isn't accessible here).

It never was (at least from the dlang.org makefile). That it had been built there was a coincidence. The entire problem happened because I wrongly assumed that the ../{dmd,druntime,phobos}-${LATEST}/ directories were never written to once cloned.

@wilzbach wilzbach force-pushed the add-dmd-to-released-pages2 branch 4 times, most recently from 3e7b829 to cab5e0c Compare June 9, 2017 08:15
@wilzbach
Copy link
Contributor Author

wilzbach commented Jun 9, 2017

Other 2 are in, rebase please

Done.

Is there another solution? Introducing this change here now will cause an inter-dependent deadlock if/when id.d is removed.
Never mind, I see this only applies to the stable snapshot.

It does apply to both stable and master, but I couldn't find a better way.
In dlang/dmd#6837 we could simply write an empty file to avoid the deadlock.

@MartinNowak
Copy link
Member

Can we please divert some more effort towards finishing dpl-docs instead of spending endless hours making the Makefile even worse.

@MartinNowak
Copy link
Member

MartinNowak commented Jun 9, 2017

Looks like id.d should be a dependency of dmd/posix.mak's html target, no?

@wilzbach
Copy link
Contributor Author

wilzbach commented Jun 9, 2017

Looks like id.d should be a dependency of dmd's html target, no?

Good idea

-> dlang/dmd#6878

Can we please divert some more effort towards finishing dpl-docs instead of spending endless hours making the Makefile even worse.

I also spend time making it better: #1687
I absolutely agree that we should push dpl-docs, but it just looks like this will require a serious time investment and for the time being showing the DMD library docs on dlang.org is sth. useful.

@CyberShadow
Copy link
Member

It does apply to both stable and master, but I couldn't find a better way.

Can you invoke the dmd makefile?

In dlang/dmd#6837 we could simply write an empty file to avoid the deadlock.

The hack should be in this repository, not dmd. In the id.d rule you can check if idgen.d exists, and if not, create an empty one.

@CyberShadow
Copy link
Member

This is a pretty big patch, can you split it up into multiple commits? And can you refactor the id.d dependency to a single place in the Makefile?

@wilzbach wilzbach force-pushed the add-dmd-to-released-pages2 branch from cab5e0c to f52c102 Compare June 9, 2017 16:14
@wilzbach wilzbach force-pushed the add-dmd-to-released-pages2 branch from f52c102 to 033ede7 Compare June 9, 2017 16:23
@wilzbach
Copy link
Contributor Author

wilzbach commented Jun 9, 2017

Can you invoke the dmd makefile?

Yes of course and this is how (1) the DMD executable is generated and (2) how the current solution works.

The hack should be in this repository, not dmd. In the id.d rule you can check if idgen.d exists, and if not, create an empty one.

I don't think it's as easy as that. Without the CTFE-generation, the entire file needs to be available when dmd -D tries to open and parse the DMD sources.

This is a pretty big patch, can you split it up into multiple commits?

Done.

And can you refactor the id.d dependency to a single place in the Makefile?

A single place is quite hard, but I realized that for the normal (master) checkouts, id.d already gets generated because we built the DMD executable, so I proposed a different way:
9e46d50

FWIW the weird thing is that we build DMD_STABLE as well (it's even a prerequisite) and the log proves this:

CC=c++ /home/dtest/DAutoTest/work/tmp/.host_dmd-2.072.2/dmd2/linux/bin64/dmd -conf=/home/dtest/DAutoTest/work/tmp/.host_dmd-2.072.2/dmd2/linux/bin64/dmd.conf -lib -of../generated/linux/release/64/lexer.a -m64 -J../generated/linux/release/64 -L-lstdc++ -version=MARS -wi ddmd/console.d ddmd/entity.d ddmd/errors.d ddmd/globals.d ddmd/id.d ddmd/identifier.d ddmd/lexer.d ddmd/tokens.d ddmd/utf.d ddmd/root/array.d ddmd/root/ctfloat.d ddmd/root/file.d ddmd/root/filename.d ddmd/root/outbuffer.d ddmd/root/port.d ddmd/root/rmem.d ddmd/root/rootobject.d ddmd/root/stringtable.d ddmd/root/hash.d
...
ddmd/arrayop.d(21): Error: module id is in file 'ddmd/id.d' which cannot be read
import path[0] = ddmd
import path[1] = /home/dtest/DAutoTest/work/tmp/.host_dmd-2.072.2/dmd2/linux/bin64/../../src/phobos
import path[2] = /home/dtest/DAutoTest/work/tmp/.host_dmd-2.072.2/dmd2/linux/bin64/../../src/druntime/import
posix.mak:564: recipe for target '/dev/shm/dtest/work/repo/dlang.org/web/phobos/ddmd_access.html' failed
make[2]: *** [/dev/shm/dtest/work/repo/dlang.org/web/phobos/ddmd_access.html] Error 1
make[2]: Leaving directory '/dev/shm/dtest/work/repo/dmd-2.074.1/src'
posix.mak:24: recipe for target 'html' failed
make[1]: *** [html] Error 2
make[1]: Leaving directory '/dev/shm/dtest/work/repo/dmd-2.074.1'
posix.mak:404: recipe for target 'dmd-release' failed

@CyberShadow
Copy link
Member

I don't think it's as easy as that. Without the CTFE-generation, the entire file needs to be available when dmd -D tries to open and parse the DMD sources.

I don't understand, can't you make the id.d target a dependency of anything that invokes said dmd -D?

Done.

Thanks a lot!

@wilzbach
Copy link
Contributor Author

wilzbach commented Jun 9, 2017

I don't understand, can't you make the id.d target a dependency of anything that invokes said dmd -D?

Isn't this what I have already done? Without modifying DMD like with dlang/dmd#6878 I need to add it to all targets that could be invoked individually, don't I?

@CyberShadow
Copy link
Member

Isn't this what I have already done? Without modifying DMD like with dlang/dmd#6878 I need to add it to all targets that could be invoked individually, don't I?

Why not:

  1. Create a target with no build steps which depends on the id.d target (and give it some abstract name)
  2. Replace all instances of id.d with that target.

My point was to avoid mentioning id.d in 5 places in the makefile. It's an "implementation detail" in a way.

Even better, I see that $(DMD_DIR) $(DMD) ${DMD_DIR}/src/ddmd/id.d are used together, why not move them to their own target which pulls in those dependencies?

@CyberShadow
Copy link
Member

I'm guessing merging dlang/dmd#6878 now wouldn't fix things short-term because the change must propagate to the release version to be usable from the dlang.org makefile?

@wilzbach
Copy link
Contributor Author

wilzbach commented Jun 9, 2017

I'm guessing merging dlang/dmd#6878 now wouldn't fix things short-term because the change must propagate to the release version to be usable from the dlang.org makefile?

Yup and hopefully soon we get the CTFE-generator..

(Will do the final touches later tonight then)

@wilzbach wilzbach force-pushed the add-dmd-to-released-pages2 branch 7 times, most recently from 2030a5b to b91c964 Compare June 10, 2017 01:26
@wilzbach
Copy link
Contributor Author

(Will do the final touches later tonight then)

Ah finally figured out the problem - it was a silly, nasty typo :S
Anyhow this makes this PR a lot less bulky and ready to ship 🎉

@CyberShadow
Copy link
Member

Looks good, what happened to the id.d rules?

@wilzbach
Copy link
Contributor Author

Looks good, what happened to the id.d rules?

There was a reference to DMD_DIR instead of DMD_STABLE_DIR somewhere (probably fixed in the unification of the DDoc variables today) and DAutoTest kept complaining. It was quite hard to find/realize, because locally, of course, everything was passing.

@CyberShadow
Copy link
Member

@dlang-bot ..?

@CyberShadow CyberShadow merged commit 5d2d478 into dlang:master Jun 10, 2017
@wilzbach wilzbach deleted the add-dmd-to-released-pages2 branch June 10, 2017 02:36
@wilzbach
Copy link
Contributor Author

@dlang-bot ..?

You just couldn't wait long enough ;-)

@CyberShadow
Copy link
Member

Usually it's instant if everything is green...

@wilzbach
Copy link
Contributor Author

Usually it's instant if everything is green...

Yes, thanks to Vibe.d the bot can do everything in less than 5ms and thus just depends on the network latency.

Hmm the bot does have write access here (I remember this being a problem before).
Debugging a bit more, I can see that GH sent the hook properly at 2017-06-10 04:35:51 (of my timezone, UTC+2):

Request URL: https://dlang-bot.herokuapp.com/github_hook
Request method: POST
content-type: application/json
Expect: 
User-Agent: GitHub-Hookshot/1c959c4
X-GitHub-Delivery: 84acdd80-4d85-11e7-994f-990a6598e2c4
X-GitHub-Event: pull_request

From the logs (converted to my timezone as well) I can't see the hook appearing there:

» 10 Jun 2017 04:35:11.746  305 <158>1 2017-06-10T02:35:11.524677+00:00 heroku router - - at=info method=POST path="/github_hook" host=dlang-bot.herokuapp.com request_id=e928890a-dfe4-47d4-a11e-730e923ae0ba fwd="192.30.252.42" dyno=web.1 connect=0ms service=4ms status=200 bytes=146 protocol=https
» 10 Jun 2017 04:35:31.460  305 <158>1 2017-06-10T02:35:31.186741+00:00 heroku router - - at=info method=POST path="/github_hook" host=dlang-bot.herokuapp.com request_id=29a46765-40fc-48c7-af30-06db4a46f4c8 fwd="192.30.252.42" dyno=web.1 connect=1ms service=3ms status=200 bytes=146 protocol=https
» 10 Jun 2017 04:35:32.281  305 <158>1 2017-06-10T02:35:31.962843+00:00 heroku router - - at=info method=POST path="/github_hook" host=dlang-bot.herokuapp.com request_id=d23a06ed-8e81-473a-b0ad-e7b3357a2d7d fwd="192.30.252.41" dyno=web.1 connect=1ms service=5ms status=200 bytes=146 protocol=https
» 10 Jun 2017 04:35:52.107  305 <158>1 2017-06-10T02:35:51.719251+00:00 heroku router - - at=info method=POST path="/github_hook" host=dlang-bot.herokuapp.com request_id=a9b7b922-6634-43f0-a83f-f57cbc3702cd fwd="192.30.252.41" dyno=web.1 connect=0ms service=3ms status=200 bytes=146 protocol=https
» 10 Jun 2017 04:36:02.413  305 <158>1 2017-06-10T02:36:02.237124+00:00 heroku router - - at=info method=POST path="/github_hook" host=dlang-bot.herokuapp.com request_id=67c0bb04-5d1e-4b16-8ee4-969066d61d9a fwd="192.30.252.41" dyno=web.1 connect=1ms service=5ms status=200 bytes=146 protocol=https
» 10 Jun 2017 04:36:10.630  305 <158>1 2017-06-10T02:36:10.229096+00:00 heroku router - - at=info method=POST path="/github_hook" host=dlang-bot.herokuapp.com request_id=5c33399d-42f0-4bdd-b4ae-973b8772e504 fwd="192.30.252.34" dyno=web.1 connect=0ms service=2ms status=200 bytes=146 protocol=https

(neither can I find the request_id at the logs).

CC @MartinNowak any ideas?

@CyberShadow
Copy link
Member

Hmm the bot does have write access here (I remember this being a problem before).

It worked in #1684 so that can't be it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants