Remove the assert->writeln pipeline in favor of the DDoc wrapper by wilzbach · Pull Request #2069 · dlang/dlang.org

wilzbach · 2018-01-09T19:18:12Z

Idea: use a custom Ddoc wrapper to pre-process the source files and avoid the
need for creating a temporary directory with the processed sources.

dlang-bot · 2018-01-09T19:18:15Z

Thanks for your pull request, @wilzbach!

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.

CyberShadow · 2018-01-09T20:52:23Z

Cool, thanks!

wilzbach · 2018-01-09T22:43:57Z

BTW any idea how I can deal with the conflicts without creating any files?

> cd ~/dlang/phobos
> cat std/algorithm/package.d | dmd -c -o- -D -
std/algorithm/comparison.d(59): Error: package name 'std.algorithm' conflicts with usage as a module name in file __stdin.d
std/algorithm/iteration.d(67): Error: package name 'std.algorithm' conflicts with usage as a module name in file __stdin.d
std/algorithm/mutation.d(78): Error: package name 'std.algorithm' conflicts with usage as a module name in file __stdin.d
std/algorithm/searching.d(106): Error: package name 'std.algorithm' conflicts with usage as a module name in file __stdin.d
std/algorithm/setops.d(48): Error: package name 'std.algorithm' conflicts with usage as a module name in file __stdin.d
std/algorithm/sorting.d(76): Error: package name 'std.algorithm' conflicts with usage as a module name in file __stdin.d

quickfur · 2018-01-09T23:08:17Z

Oh man... looks like the hack I used to make stdin work with dmd has backfired. :-( Will have to look into this deeper at some point.

wilzbach · 2018-01-10T13:40:45Z

Started to move ahead with this transition and moved the second commit to #2072

wilzbach · 2018-02-04T22:04:21Z

@CyberShadow I think this is finally ready :)

Note: it has the downsides

that package modules don't use the assert/rewrite pipeline for now
temporary files are used/created for now
ddox doesn't use this assert/rewrite pipeline either

However, the assert/writeln rewrite is imho just a nice to have - it's not a huge deal if it's not there for a few modules and on the upside:

we can now easily enable it for druntime/dmd
we get rid of -80 lines
it makes disabling the assert/rewrite pipeline a lot easier (could be a simple flag for the CLI preprocessor)
we get rid of the annoying need to auto-clone phobos

Especially the last bit would be nice to finally have - it would make #2162 a bit easier.

Once this is in, we can always figure out how to close the remaining problems, but I think we need to have a start and common base somewhere.

CyberShadow · 2018-02-04T22:43:33Z

A bit sad that the DDox examples will regress to standard asserts, but I guess that could be fixable in DDox itself, right?

CyberShadow

You still have a reference to $(ASSERT_WRITELN_BIN), that's causing the test target to fail:

test: $(ASSERT_WRITELN_BIN)_test test_dspec test/next_version.sh all

CyberShadow · 2018-02-04T22:45:29Z

+#  before actually running Ddoc.
+#  Currently this is used for:
+#    - TOC
+#    - assert -> writeln magic


Seems like the two lists of the program's tasks/capabilities are redundant (and one is more complete than the other).

👍 on more docs though.

CyberShadow · 2018-02-04T22:45:41Z

+#  - This transforms assert(a == b) to writeln(a); // b
+#  - It creates a copy of Phobos to apply the transformations
+#  - All "d" files are piped through the transformator,
+#   other needed files (e.g. posix.mak) get copied over


(the second list is here)

CyberShadow · 2018-02-04T22:49:21Z

@@ -0,0 +1,4 @@
+name "ddoc_preprocessor"
+description "Preprocessos source code before running Ddoc over it"


"Preprocesses" :)

CyberShadow · 2018-02-04T22:51:09Z

+name "ddoc_preprocessor"
+description "Preprocessos source code before running Ddoc over it"
+dependency "libdparse" version="0.7.2-alpha.4"
+targetType "executable"


Do we have do use Dub here?

Unless you plan to add dependencies from code.dlang.org, rdmd should suffice.

I don't object to using Dub, just thinking back to all the times it gave us trouble, and don't want to add to that pile.

Fair point, but it doesn't change the status quo. The assert_writeln_magic script used dub too.

Unless you plan to add dependencies from code.dlang.org

Yeah, we need libdparse. I'm no huge fan of dub either, either because it seems like we/I are the ones who do all the dog-fooding of it to make it and the registry mature.
Unfortunately, that's also a reason to keep using dub here (instead of doing git clones).

wilzbach · 2018-02-05T00:12:46Z

A bit sad that the DDox examples will regress to standard asserts, but I guess that could be fixable in DDox itself, right?

Okay, now I did it in this PR ;-)
I had to copy over the ddox main executable even though only a small patch is required - see the last commit: 93133cf

CyberShadow · 2018-02-05T00:25:08Z

Failing with make: dub: Command not found

@wilzbach Not really related to this PR, but it looks like DAutoTest is redoing a lot of work for the test target. This is probably contributing a lot to the long build times. E.g. open a log and search for dautotest: Running tests - there's a lot of stuff past that point that seems redundant.

wilzbach · 2018-02-05T00:57:45Z

there's a lot of stuff past that point that seems redundant.

Good catch - let's try #2170 for a start

wilzbach · 2018-02-08T22:42:28Z

(rebased)

Are the changes to ddox.main upstreamable? This might not be a future-proof approach if ddox gets updated.

Not sure, while it's essentially a two-line change, ddox isn't very customizable yet.
I will try to get them upstream, but that might take weeks or months.

wilzbach · 2018-02-09T09:34:10Z

Rebased again - it's getting a bit tedious to maintain/rebase this. What's blocking for this?

CyberShadow · 2018-02-09T09:35:13Z

I've been a little hesitant because forking DDox seems to be a liability in the long-term.

Would be good to see some initial effort of upstreaming the changes before this is merged.

wilzbach · 2018-02-15T17:19:47Z

https://issues.dlang.org/show_bug.cgi?id=18442 (linking to unittests) would be easy to implement with the changes of this PR.

…tches)

wilzbach · 2018-02-19T05:24:48Z

I've been a little hesitant because forking DDox seems to be a liability in the long-term.
Would be good to see some initial effort of upstreaming the changes before this is merged.

Okay, I reduced the copy of ddox.main to the currently required minimum and opened a PR with the change that would be required to remove "the fork":

dlang/ddox#199

Good enough?

CyberShadow

Thank you! Hopefully this will go a long way.

The first process was started by `pipeProcess` but never interacted with, not even calling `wait` before starting the next instance - with exact same arguments! Seems like this line wasn't deleted in dlang#2069 which introduced `execute` and removed the other code interacting with the piped process.

wilzbach requested review from CyberShadow, MartinNowak and andralex as code owners January 9, 2018 19:18

wilzbach mentioned this pull request Jan 14, 2018

Rename ddoc.d -> ddoc_preprocessor.d #2082

Merged

wilzbach force-pushed the writeln-dynamic branch from 178cd57 to 74fb6ab Compare February 4, 2018 21:27

wilzbach removed the WIP label Feb 4, 2018

wilzbach force-pushed the writeln-dynamic branch from 74fb6ab to 4ca26aa Compare February 4, 2018 21:30

wilzbach changed the title ~~[WIP] Remove the assert->writeln pipeline in favor of the DDoc wrapper~~ Remove the assert->writeln pipeline in favor of the DDoc wrapper Feb 4, 2018

wilzbach force-pushed the writeln-dynamic branch 3 times, most recently from 5078a84 to 3c031c1 Compare February 4, 2018 22:02

wilzbach mentioned this pull request Feb 4, 2018

Checkout upstream/stable instead of LATEST #2162

Open

wilzbach added the Blocking other work label Feb 4, 2018

wilzbach force-pushed the writeln-dynamic branch from 3c031c1 to 8aaf8a8 Compare February 4, 2018 22:17

CyberShadow reviewed Feb 4, 2018

View reviewed changes

wilzbach force-pushed the writeln-dynamic branch 2 times, most recently from 795b293 to 93133cf Compare February 5, 2018 00:08

wilzbach force-pushed the writeln-dynamic branch 2 times, most recently from b8aa500 to e72e872 Compare February 5, 2018 00:35

wilzbach force-pushed the writeln-dynamic branch from e72e872 to dcbaf50 Compare February 5, 2018 01:05

wilzbach force-pushed the writeln-dynamic branch from e840a84 to edc0f5b Compare February 9, 2018 09:32

wilzbach force-pushed the writeln-dynamic branch from edc0f5b to 4fee57e Compare February 13, 2018 00:11

wilzbach added 11 commits February 19, 2018 03:22

Dynamic writeln magic

8267493

Pass Phobos Ddoc files through DDOC_BIN

7b4d373

Move files to ddoc

53ec71d

Integrate the assert_writeln_magic in the preprocessor

12cb659

Add main from ddox

1cbf1c8

Use assert_writeln_magic in dpl-docs

ceeeccb

Improve auto-macro injection

f2caa15

Use the assert/writeln pipeline for the verbatim files

66114c2

Imitate package directory layout for package.d files

7797e5c

Temporarily fix broken Ddoc in std.exception.enforceEx

e2f21b4

Add dmd as a dependency to the preprocessor (required for the CLI swi…

f3f3d6a

…tches)

wilzbach force-pushed the writeln-dynamic branch from 4fee57e to f3f3d6a Compare February 19, 2018 02:23

Use --compiler for the verbatim documentation build

1125004

wilzbach force-pushed the writeln-dynamic branch from c73a7dd to 1125004 Compare February 19, 2018 04:21

wilzbach mentioned this pull request Feb 19, 2018

Move commands from ddox/main.d to ddox/commands.d dlang/ddox#198

Closed

Use static import to avoid code duplication from ddox.main

474fb07

wilzbach mentioned this pull request Feb 19, 2018

Allow a custom postprocessor for FilterDocs dlang/ddox#199

Open

CyberShadow approved these changes Feb 19, 2018

View reviewed changes

wilzbach added the auto-merge label Feb 19, 2018

wilzbach deleted the writeln-dynamic branch February 19, 2018 07:05

wilzbach mentioned this pull request Mar 20, 2018

Remove auto-cloning of Phobos on start #2286

Merged

MoonlightSentinel mentioned this pull request Feb 12, 2022

ddoc_preprocessor: Don't execute dmd twice #3216

Merged

		@@ -0,0 +1,4 @@
		name "ddoc_preprocessor"
		description "Preprocessos source code before running Ddoc over it"

Uh oh!

Uh oh!

Conversation

wilzbach commented Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlang-bot commented Jan 9, 2018

Bugzilla references

Uh oh!

CyberShadow commented Jan 9, 2018

Uh oh!

wilzbach commented Jan 9, 2018

Uh oh!

quickfur commented Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Jan 10, 2018

Uh oh!

wilzbach commented Feb 4, 2018

Uh oh!

CyberShadow commented Feb 4, 2018

Uh oh!

CyberShadow left a comment

Choose a reason for hiding this comment

Uh oh!

CyberShadow Feb 4, 2018

Choose a reason for hiding this comment

Uh oh!

CyberShadow Feb 4, 2018

Choose a reason for hiding this comment

Uh oh!

CyberShadow Feb 4, 2018

Choose a reason for hiding this comment

Uh oh!

CyberShadow Feb 4, 2018

Choose a reason for hiding this comment

Uh oh!

wilzbach Feb 4, 2018

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Feb 5, 2018

Uh oh!

CyberShadow commented Feb 5, 2018

Uh oh!

wilzbach commented Feb 5, 2018

Uh oh!

wilzbach commented Feb 8, 2018

Uh oh!

wilzbach commented Feb 9, 2018

Uh oh!

CyberShadow commented Feb 9, 2018

Uh oh!

wilzbach commented Feb 15, 2018

Uh oh!

wilzbach commented Feb 19, 2018

Uh oh!

CyberShadow left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wilzbach commented Jan 9, 2018 •

edited

Loading

quickfur commented Jan 9, 2018 •

edited

Loading