Skip to content

Convert DUB package test into a Makefile test and run it with the proper compiler flags #6999

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.

Sign up for GitHub

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

Closed
wilzbach wants to merge 1 commit into from
Closed

Convert DUB package test into a Makefile test and run it with the proper compiler flags #6999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion dub.sdl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ authors "Walter Bright"
copyright "Copyright © 1999-2017, Digital Mars"
license "BSL-1.0"

targetType "none"
targetType "library"
Copy link
Copy Markdown
Contributor

@jacob-carlborg jacob-carlborg Jul 17, 2017

Choose a reason for hiding this comment

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

I was told by Sönke to not do this #6771 (comment).

Copy link
Copy Markdown
Contributor Author

@wilzbach wilzbach Jul 17, 2017

Choose a reason for hiding this comment

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

It doesn't copy all DFLAGS otherwise.

Copy link
Copy Markdown
Contributor

@jacob-carlborg jacob-carlborg Jul 17, 2017

Choose a reason for hiding this comment

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

Hmm, ok.

Copy link
Copy Markdown
Member

@MartinNowak MartinNowak Jul 26, 2017

Choose a reason for hiding this comment

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

Who doesn't copy what DFLAGS?

dependency ":parser" version="*"
sourcePaths

subPackage {
name "root"
Expand Down
18 changes: 15 additions & 3 deletions test/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ QUIET=@
export RESULTS_DIR=test_results
export MODEL
export REQUIRED_ARGS=
Copy link
Copy Markdown
Member

@MartinNowak MartinNowak Jul 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

BTW, working with exported env parameters is really annoying when reading a log failure and not being able to rerun a command.

BUILD=release
Copy link
Copy Markdown
Contributor

@jacob-carlborg jacob-carlborg Jul 16, 2017

Choose a reason for hiding this comment

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

Is this used?

Copy link
Copy Markdown
Contributor Author

@wilzbach wilzbach Jul 17, 2017

Choose a reason for hiding this comment

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

Yep, I had to do quite a few try to figure out a working setup and in the one you reviewed I temporarily removed the new changes with $(BUILD)


ifeq ($(findstring win,$(OS)),win)
export ARGS=-inline -release -g -O
Expand All @@ -112,19 +113,28 @@ export DFLAGS=-I$(DRUNTIME_PATH)\import -I$(PHOBOS_PATH)
export LIB=$(PHOBOS_PATH)
else
export ARGS=-inline -release -g -O -fPIC
export DMD=../src/dmd
# hack around the fact that on auto-tester MODEL isn't set for the testsuite
Copy link
Copy Markdown
Member

@CyberShadow CyberShadow Jul 17, 2017

Choose a reason for hiding this comment

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

That doesn't sound right.

https://github.com/braddr/at-client/blob/master/src/do_test_dmd.sh#L33-L37

There's certainly a MODEL parameter there.

Generally, if things can be fixed in the AT scripts, they should be. I think Brad has generally been responsive in merging AT pull requests as long as they don't require more involved things like merging some pull requests simultaneously or manually updating test machines.

Copy link
Copy Markdown
Contributor Author

@wilzbach wilzbach Jul 17, 2017

Choose a reason for hiding this comment

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

As mentioned I have bumped into this before: #6870
And no I don't think that we want to wait for another one or two months here. If this gets fixed at the auto-tester's side, the workaround can easily be removed.

Copy link
Copy Markdown
Member

@MartinNowak MartinNowak Jul 26, 2017

Choose a reason for hiding this comment

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

Yes that looks wrong, osmodel.mak will already determine a default MODEL, unless one is explicity specified via make MODEL=64. Not sure what fails for you, but you can use $(error ${MODEL}) to inspect variables.

ifeq (,$(wildcard ../generated/$(OS)/$(BUILD)/64/dmd))
REAL_MODEL=32
else
REAL_MODEL=64
endif
export DMD=../generated/$(OS)/$(BUILD)/$(REAL_MODEL)/dmd
Copy link
Copy Markdown
Contributor Author

@wilzbach wilzbach Jul 17, 2017

Choose a reason for hiding this comment

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

I already bumped into this before: #6870
This is required as dub doesn't forward the -L-L DFLAGS to the linker (in this case dmd again) and thus we need the dmd.conf, which isn't generated for src/dmd by default anymore.

Linking...
/home/seb/dlang/dmd/generated/linux/release/64/dmd -of.dub/build/application-$DFLAGS-linux.posix-x86_64-dmd_2075-1833BAD8BC30CEBFE35D7CC6DA0FFC50/dmd-dub-test .dub/build/application-$DFLAGS-linux.posix-x86_64-dmd_2075-1833BAD8BC30CEBFE35D7CC6DA0FFC50/dmd-dub-test.o ../../.dub/build/library-$DFLAGS-linux.posix-x86_64-dmd_2075-3768A66D24D1220DBE5011CA9AC1DEA7/libdmd_parser.a ../../.dub/build/library-$DFLAGS-linux.posix-x86_64-dmd_2075-00BFCF605E8D9C274B8361919B5DB43D/libdmd_lexer.a ../../.dub/build/library-$DFLAGS-linux.posix-x86_64-dmd_2075-831C9FFAA175BCEDD4C554013935344A/libdmd_root.a -L--no-as-needed -m64 -defaultlib=libphobos2.so
Copying target from /home/seb/dlang/dmd/test/dub_package/.dub/build/application-$DFLAGS-linux.posix-x86_64-dmd_2075-1833BAD8BC30CEBFE35D7CC6DA0FFC50/dmd-dub-test to /home/seb/dlang/dmd/test/dub_package
Running ./dmd-dub-test 
./dmd-dub-test: error while loading shared libraries: libphobos2.so.0.75: cannot open shared object file: No such file or directory

Copy link
Copy Markdown
Member

@CyberShadow CyberShadow Jul 17, 2017

Choose a reason for hiding this comment

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

Wait, so you want to build the Dub library with the built DMD, not the host DMD?

I guess testing that the compiler can auto-bootstrap is interesting, but wouldn't it be easier and more consistent to use the host DMD? Or is that difficult because only dmd's posix.mak knows what it is?

Copy link
Copy Markdown
Contributor Author

@wilzbach wilzbach Jul 17, 2017

Choose a reason for hiding this comment

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

to use the host DMD

We should test both, but this entire issue exits because gdc's fronted is too old and has bugs.

Copy link
Copy Markdown
Member

@MartinNowak MartinNowak Jul 26, 2017

Choose a reason for hiding this comment

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

As the goal is to check whether the dub package recipe works we should just use the host compiler and avoid additional complications.

export EXE=
export OBJ=.o
export DSEP=/
export SEP=/

DRUNTIME_PATH=../../druntime
PHOBOS_PATH=../../phobos
DUB=dub
# Default DUB flags (DUB uses a different architecture format)
DUBFLAGS=--compiler=$(abspath $(DMD)) --arch=$(subst 32,x86,$(subst 64,x86_64,$(REAL_MODEL)))
# link against shared libraries (defaults to true on supported platforms, can be overridden w/ make SHARED=0)
SHARED=$(if $(findstring $(OS),linux freebsd),1,)
DFLAGS=-I$(DRUNTIME_PATH)/import -I$(PHOBOS_PATH) -L-L$(PHOBOS_PATH)/generated/$(OS)/release/$(MODEL)
DFLAGS=-I$(abspath $(DRUNTIME_PATH))/import -I$(abspath $(PHOBOS_PATH)) -L-L$(abspath $(PHOBOS_PATH))/generated/$(OS)/release/$(MODEL) -fPIC
ifeq (1,$(SHARED))
DFLAGS+=-defaultlib=libphobos2.so -L-rpath=$(PHOBOS_PATH)/generated/$(OS)/release/$(MODEL)
DFLAGS+=-defaultlib=libphobos2.so -L-rpath=$(abspath $(PHOBOS_PATH))/generated/$(OS)/release/$(MODEL)
endif
export DFLAGS
endif
Expand Down Expand Up @@ -209,3 +219,5 @@ $(RESULTS_DIR)/d_do_test$(EXE): d_do_test.d $(RESULTS_DIR)/.created
$(QUIET)$(DMD) -conf= $(MODEL_FLAG) -unittest -run d_do_test.d -unittest
$(QUIET)$(DMD) -conf= $(MODEL_FLAG) -od$(RESULTS_DIR) -of$(RESULTS_DIR)$(DSEP)d_do_test$(EXE) d_do_test.d

test_dub:
cd ./dub_package && $(DUB) test $(DUBFLAGS) --force -v
11 changes: 1 addition & 10 deletions travis.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,14 @@ rebuild() {

# test druntime, phobos, dmd
test() {
# Temporarily skip testing the DUB package
#See also: https://github.com/dlang/dmd/pull/6999
#test_dub_package
make -j$N -C ../druntime -f posix.mak MODEL=$MODEL unittest
make -j$N -C ../phobos -f posix.mak MODEL=$MODEL unittest
test_dmd
}

# test dmd
test_dmd() {
make -j$N -C test test_dub MODEL=$MODEL
# test fewer compiler argument permutations for PRs to reduce CI load
if [ "$TRAVIS_PULL_REQUEST" == "false" ] && [ "$TRAVIS_OS_NAME" == "linux" ]; then
make -j$N -C test MODEL=$MODEL # all ARGS by default
Expand All @@ -72,13 +70,6 @@ test_dmd() {
fi
}

# test dub package
test_dub_package() {
pushd test/dub_package
dub test
popd
}

for proj in druntime phobos; do
if [ $TRAVIS_BRANCH != master ] && [ $TRAVIS_BRANCH != stable ] &&
! git ls-remote --exit-code --heads https://github.com/dlang/$proj.git $TRAVIS_BRANCH > /dev/null; then
Expand Down