Skip to content

fix support of relative variable overrides#1771

Merged
dlang-bot merged 2 commits intodlang:stablefrom
MartinNowak:fix_relative_overrides
Jun 26, 2017
Merged

fix support of relative variable overrides#1771
dlang-bot merged 2 commits intodlang:stablefrom
MartinNowak:fix_relative_overrides

Conversation

@MartinNowak
Copy link
Member

No description provided.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jun 25, 2017

Thanks for your pull request, @MartinNowak!

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.

@MartinNowak MartinNowak requested a review from wilzbach June 25, 2017 13:40
@wilzbach
Copy link
Contributor

make[1]: Leaving directory '/dev/shm/dtest/work/repo/dmd/src'
make: *** No rule to make target '../druntime-2.074.1', needed by '.generated/modlist-2.074.1.ddoc'. Stop.

Hmm the git clone targets are absolute (due to $(PWD)) as well.
How about replacing the first git clone rule with sth. like this?

${DMD_DIR}-${LATEST} ${DRUNTIME_DIR}-${LATEST} ${PHOBOS_DIR}-${LATEST} ${TOOLS_DIR}-${LATEST} ${INSTALLER_DIR}-${LATEST}:
	git clone --depth=1 -b v${LATEST} ${GIT_HOME}/$(word 1, $(subst -, ,$(notdir $(@F)))) $@

posix.mak Outdated
STDDOC="$(addprefix $(PWD)/, $(STD_DDOC))" \
DMD="$(DMD_STABLE)" \
DRUNTIME_PATH="${DRUNTIME_DIR}" \
DMD="$(realpath $(DMD_STABLE))" \
Copy link
Member

Choose a reason for hiding this comment

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

Please no realpath, it resolves symlinks, and the final paths (which may be temporary and expose implementation details) end up all over the place, sometimes even in outputs.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's what we've used during the previous years, but I could use make's abspath if that helps.

Copy link
Member

Choose a reason for hiding this comment

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

Yes please.

@MartinNowak
Copy link
Member Author

Hmm the git clone targets are absolute (due to $(PWD)) as well.
How about replacing the first git clone rule with sth. like this?

This is unrelated to this change and was introduced here #1687 (comment).

@wilzbach
Copy link
Contributor

This is unrelated to this change and was introduced here #1687 (comment).

Maybe, but it would fix the broken build on DAutoTest.

@MartinNowak
Copy link
Member Author

See #1772 @wilzbach.
And again, I'd rather we spent time to finish the last few missing bits for ddox than throwing even more at the ever more complex ddoc build.

@wilzbach wilzbach mentioned this pull request Jun 26, 2017
- broken by 4129e45 which changed target folders from
  ../druntime-2.074.1 to $PWD/druntime-2.074.1
- no need to make those targets absolute since the makefile will
  always run in the dlang.org root folder
@MartinNowak MartinNowak force-pushed the fix_relative_overrides branch from f127679 to 2efc3fd Compare June 26, 2017 11:06
@MartinNowak
Copy link
Member Author

Also fixes the clone targets now.

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

Thanks!

@wilzbach
Copy link
Contributor

MartinNowak wants to merge 2 commits into dlang:stable

Btw as master gets auto-deployed on dlang.org, we could also declare stable as default branch for dlang.org if that makes your life easier?

@dlang-bot dlang-bot merged commit 8d3ec1b into dlang:stable Jun 26, 2017
@MartinNowak MartinNowak deleted the fix_relative_overrides branch June 28, 2017 09:12
@MartinNowak
Copy link
Member Author

Btw as master gets auto-deployed on dlang.org, we could also declare stable as default branch for dlang.org if that makes your life easier?

It's fine for the moment and not much effort.

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.

4 participants