Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove hard-coded src/dmd executables #4

Merged
merged 1 commit into from
Dec 5, 2017

Conversation

wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Oct 5, 2017

Trying to fix https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=2816476&dataid=19934733&isPull=true

DMD gets set by default by all these Makefile nowadays and removing it is a lost easier than recreating the $(DMD_DIR)/generated/$(OS)/release/$(MODEL)/dmd logic.

See also: dlang/dmd#7135

@wilzbach wilzbach mentioned this pull request Oct 5, 2017
15 tasks
@braddr
Copy link
Owner

braddr commented Oct 6, 2017

I'd be fine with using the defaults if they were the same or correct value. The problem is that they're only the same on posix and incorrect on both win32 and win64, unless I'm misreading things. The two windows makefiles end up just using the search path, which is too uncontrolled to be a good default. For the auto-testers, there's no version of dmd in the path at all, on purpose.

So, address these problems for win32 and win64 and then we can move forward with removing this explicit setting of DMD in the scripts.

@wilzbach
Copy link
Contributor Author

So, address these problems for win32 and win64 and then we can move forward with removing this explicit setting of DMD in the scripts.

Okay I submitted dlang/druntime#1978 and dlang/phobos#5892

Also I submitted another PR (#5) which instead of removing the DMD paths, sets them to the new generated path which I believe is the easier & faster way to go?

@braddr
Copy link
Owner

braddr commented Nov 29, 2017

I prefer to remove the hard coding than just changing it. Get the two pull requests above merged to the staging branch then I can merge this in.

@wilzbach
Copy link
Contributor Author

Get the two pull requests above merged to the staging branch then I can merge this in.

They are both in 🎉
So we should be able to move forward here?

@braddr
Copy link
Owner

braddr commented Nov 29, 2017

Sorry, not staging, but stable. All tested branches need to have the changes. In fact, since I merged the auto-tester changes in before remembering to check stable, it's currently failing to build. That needs to be fixed asap or I'll have to revert the auto-tester or put in a temporary work around. It'd be best to just get the changes merged.

@wilzbach
Copy link
Contributor Author

Sorry, not staging, but stable. All tested branches need to have the changes. In fact, since I merged the auto-tester changes in before remembering to check stable, it's currently failing to build. That needs to be fixed asap or I'll have to revert the auto-tester or put in a temporary work around. It'd be best to just get the changes merged.

PRs for druntime & phobos targeting stable:

dlang/druntime#1979
dlang/phobos#5895

@wilzbach
Copy link
Contributor Author

wilzbach commented Nov 30, 2017

In fact, since I merged the auto-tester changes in before remembering to check stable, it's currently failing to build.

Thanks a lot for merging the PRs to stable so quickly, but looking at current PRs I still see the following on the auto-tester:

build phobos:

make -f posix.mak OS=linux MODEL=32 BUILD=release INSTALL_DIR=../install \
	DMD=../dmd/src/dmd install2
make[1]: Entering directory '/home/braddr/sandbox/at-client/pull-2917325-Linux_32/phobos'
make -C ../druntime -f posix.mak MODEL=32 DMD=../dmd/src/dmd OS=linux BUILD=release

test druntime

make -f posix.mak unittest OS=linux MODEL=32 DMD=../dmd/src/dmd BUILD=debug
make -f posix.mak unittest OS=linux MODEL=32 DMD=../dmd/src/dmd BUILD=release

test phobos:

make -f posix.mak unittest OS=linux MODEL=32 DMD=../dmd/src/dmd BUILD=debug
make -f posix.mak unittest OS=linux MODEL=32 DMD=../dmd/src/dmd BUILD=release

https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=2917325&dataid=20643200&isPull=true

Do I interpret this correctly that this change isn't deployed yet?

@braddr braddr merged commit 6f7ebc7 into braddr:master Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants