Use Makefile's pattern rules for the intermediate assert/writeln translation #1590

wilzbach · 2017-02-27T20:39:54Z

only regeneration when real changes happen
removes the ugly rsync workaround

CyberShadow · 2017-03-01T08:09:11Z

assert_writeln_magic.d

+ {
+ auto tmpFile = File(destFile ~ ".tmp", "w");
+ writeLinesToFile(tmpFile);
+ tmpFile.name.rename(destFile);


Renaming an open file will fail on Windows. Do we care about that?

Actually, I think it won't. Renaming over an open file will. Never mind.

CyberShadow · 2017-03-01T08:12:37Z

assert_writeln_magic.d

+ if (inputDir.length == 0)
+ parseFile(file.name, outputDir);
+ else
+ parseFile(file.name, file.name.replace(inputDir, outputDir));


Tricky! What if inputDir is "."?

For my own use I have https://github.com/CyberShadow/ae/blob/13bc434ea12c90883f524818510f35970230becb/utils/path.d#L18-L23

CyberShadow · 2017-03-01T08:17:59Z

posix.mak

@@ -40,6 +40,10 @@ PHOBOS_STABLE_DIR=${PHOBOS_DIR}-${LATEST}
 GENERATED=.generated
 PHOBOS_DIR_GENERATED=$(GENERATED)/phobos-prerelease
 PHOBOS_STABLE_DIR_GENERATED=$(GENERATED)/phobos-release
+PHOBOS_FILES := $(shell find $(PHOBOS_DIR) -name '*.d' -o -name '*.mak' -o -name '*.ddoc')
+PHOBOS_FILES_GENERATED := $(subst $(PHOBOS_DIR), $(PHOBOS_DIR_GENERATED), $(PHOBOS_FILES))


In general it would be nice if more things in the makefile were documented.

wilzbach · 2017-03-01T16:42:30Z

In general it would be nice if more things in the makefile were documented

I gave it a first shot. Nothing fancy, the biggest changes were to use the same osmodel.mak as everywhere else and the removal of the two git clones rules in favor of an already existing, generic one.

wilzbach · 2017-03-01T16:43:57Z

assert_writeln_magic.d

+ if (inputDir.length == 0)
+ parseFile(file.name, outputDir);
+ else
+ parseFile(file.name, file.name.rebasePath(inputDir, outputDir));


btw I just kept this support because it might be nice "in a possible future". It's not needed anymore as the Makefile does the invocations with a single file.

wilzbach · 2017-03-01T16:45:05Z

posix.mak

@@ -33,13 +39,24 @@ [email protected]:data

 # Last released versions
 DMD_STABLE_DIR=${DMD_DIR}-${LATEST}
+DMD_REL=$(DMD_STABLE_DIR)/src/dmd


Imho it would be better to rename this existing variable to sth. like DMD_STABLE_BIN

wilzbach · 2017-03-01T16:46:51Z

posix.mak

 ################################################################################

-$(GENERATED):
- mkdir -p $@
+ASSERT_WRITELN_BIN = $(GENERATED)/assert_writeln_magic


Yet another small cleanup - sorry that I didn't think about this earlier.

CyberShadow · 2017-03-01T16:47:15Z

I gave it a first shot.

Great, thanks!

CyberShadow · 2017-03-01T16:48:50Z

BTW since I noticed [SQUASH] in commit titles seems to be a pattern, do you know about git commit --fixup?

wilzbach · 2017-03-01T16:49:59Z

posix.mak

-HAS_RSYNC := $(shell command -v rsync 2> /dev/null)
+$(ASSERT_WRITELN_BIN): assert_writeln_magic.d $(DUB)
+ @mkdir -p $(dir $@)
+ $(DUB) build --single $<


Note that using DUB without a specified DMD executable apparently broke Martin's build script.
This is the current "status quo" and I am waiting for his feedback on how to resolve this at #1592
(so this shouldn't block this PR and #1592 can easily be rebased)

wilzbach · 2017-03-01T16:53:36Z

BTW since I noticed [SQUASH] in commit titles seems to be a pattern, do you know about git commit --fixup?

Yep, but I don't want to force-push during a review. The idea is that you only have to review the new changes ;-)

CyberShadow · 2017-03-01T16:57:05Z

--fixup still creates a new commit, it just makes rebasing easier later.

wilzbach · 2017-03-01T17:59:37Z

--fixup still creates a new commit, it just makes rebasing easier later.

Not sure if it makes a difference if we use auto-merge-squash anyways later ;-)

Btw anything blocking this? I think this time I should do manual squashing to keep some commits, so please let me know.

CyberShadow · 2017-03-01T18:00:48Z

Not sure if it makes a difference if we use auto-merge-squash anyways later ;-)

Actually, would be cool if dbot could auto-squash --fixup commits.

CyberShadow · 2017-03-01T18:02:05Z

I think this time I should do manual squashing to keep some commits, so please let me know.

Yes please.

- only regeneration - removes the ugly rsync workaround

wilzbach · 2017-03-01T18:15:20Z

Actually, would be cool if dbot could auto-squash --fixup commits.

Good idea - noted it here:

dlang/dlang-bot#64

(but the queue is currently a bit full).

Yes please.

Done :)

CyberShadow · 2017-03-01T18:16:44Z

Well, I was going to merge anyway :P

wilzbach changed the title ~~Use pattern rules for the intermediate assert/writeln translation~~ Use Makefile's pattern rules for the intermediate assert/writeln translation Feb 28, 2017

CyberShadow reviewed Mar 1, 2017

View reviewed changes

wilzbach commented Mar 1, 2017

View reviewed changes

CyberShadow approved these changes Mar 1, 2017

View reviewed changes

wilzbach added 5 commits March 1, 2017 19:07

Use pattern rules for the intermediate assert/writeln translation

00aa0bb

- only regeneration - removes the ugly rsync workaround

Add more comments to posix.mak

e20cfee

Use osmodel.mak for dlang.org

83b47d6

Add tools and installer dir to the auto git cloned repos

ca6ba64

write the assert_writeln_magic binary to the .generated folder

c9cf21e

wilzbach force-pushed the cleanup-makefile-patterns branch from 2f61f77 to c9cf21e Compare March 1, 2017 18:07

wilzbach merged commit c1e1666 into dlang:master Mar 1, 2017

wilzbach deleted the cleanup-makefile-patterns branch March 1, 2017 18:15

MartinNowak mentioned this pull request Apr 10, 2017

Merge remote-tracking branch 'upstream/stable' into merge_stable #1629

Merged

wilzbach mentioned this pull request May 10, 2017

Ensure that the phobos-stable directory exists #1650

Merged

CyberShadow mentioned this pull request Jun 7, 2017

Revert some PRs, fix master #1682

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Makefile's pattern rules for the intermediate assert/writeln translation #1590

Use Makefile's pattern rules for the intermediate assert/writeln translation #1590

wilzbach commented Feb 27, 2017

CyberShadow Mar 1, 2017

CyberShadow Mar 1, 2017

CyberShadow Mar 1, 2017

CyberShadow Mar 1, 2017

wilzbach commented Mar 1, 2017

wilzbach Mar 1, 2017

wilzbach Mar 1, 2017 •

edited

Loading

wilzbach Mar 1, 2017

CyberShadow commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

wilzbach Mar 1, 2017

wilzbach commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

wilzbach commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

wilzbach commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

Use Makefile's pattern rules for the intermediate assert/writeln translation #1590

Use Makefile's pattern rules for the intermediate assert/writeln translation #1590

Conversation

wilzbach commented Feb 27, 2017

CyberShadow Mar 1, 2017

Choose a reason for hiding this comment

CyberShadow Mar 1, 2017

Choose a reason for hiding this comment

CyberShadow Mar 1, 2017

Choose a reason for hiding this comment

CyberShadow Mar 1, 2017

Choose a reason for hiding this comment

wilzbach commented Mar 1, 2017

wilzbach Mar 1, 2017

Choose a reason for hiding this comment

wilzbach Mar 1, 2017 • edited Loading

Choose a reason for hiding this comment

wilzbach Mar 1, 2017

Choose a reason for hiding this comment

CyberShadow commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

wilzbach Mar 1, 2017

Choose a reason for hiding this comment

wilzbach commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

wilzbach commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

wilzbach commented Mar 1, 2017

CyberShadow commented Mar 1, 2017

wilzbach Mar 1, 2017 •

edited

Loading