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

Failure of the latest Windows builds #17315

Closed
illi-kun opened this issue Jun 23, 2016 · 15 comments

Comments

Projects
None yet
7 participants
@illi-kun
Copy link
Member

commented Jun 23, 2016

Jenkins builds fail with the following console output:

...
obj/visitable_remove.o:visitable_remove.cpp:(.text+0x1894): undefined reference to `item::in_its_container()'
collect2: error: ld returned 1 exit status
make[1]: *** [cata_test] Error 1
make[1]: Leaving directory `/var/lib/jenkins/cataclysm-reference-repo/Windows-Curses/tests'
make: *** [tests] Error 2
...
Build step 'Execute shell' marked build as failure
@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2016

Build 5081 may have been the first

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2016

By the way, why does Jenkins care about tests? I don't think they're supplied with the executable.

We should probably have a make switch that disables tests. And if we do have one already, Jenkins should invoke it.

@illi-kun

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2016

Jenkins cares about that for bringing developers' attention to (potencial) issues highlighted by the failed tests, at least I guess so.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2016

It should recover from mistakes when it's possible. It can announce a failed build status on all PRs, but it should still produce the regular executable anyway.

@illi-kun

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2016

It sounds reasonable.

@Zireael07

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2016

I need my Cataclysm fix. Any fix for this coming?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2016

I don't understand how does this bug happen.
Could it be that the compiler inlines in_its_container and removes it from the object file?
The function is very simple (literally just return in_container( type->default_container );).

If this is the case, guaranteeing that tests always build may be a pain, so we really need to make Jenkins understand "partial failure" better.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2016

I didn't fix the builds, but at least I changed the error. Now it's one that can't be fixed with PRs alone.

w64-mingw32.static-g++  -DRELEASE -DCROSS_LINUX -DGIT_VERSION -DLOCALIZE -ffast-math -Os  -Wall -Wextra   -Werror -std=c++11 -MMD  -DLUA -c src/version.cpp -o objwin/version.o
make: /home/narc/mxe/usr/bin/i686-w64-mingw32.static-ccache: Command not found

I'm pretty sure only @narc0tiq can fix it.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Jun 26, 2016

Has... has our mingw32 compiler gone AWOL?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jun 26, 2016

It should recover from mistakes when it's possible. It can announce a failed build status on all PRs, but it should still produce the regular executable anyway.

Nope, that removes any pressure whatsoever from keeping the tests building, then they bitrot and become useless. If nobody cares enough to keep tests building on a platform, we're evidently not supporting that platform anymore.

This particular error is quite odd, it's trying to link the tests before doing anything else.
I suspect this might be the actual problem: http://ci.narc.ro/job/Cataclysm-Matrix/Graphics=Curses,Platform=Windows_x64/5082/console

ccache /home/narc/mxe/usr/bin/x86_64-w64-mingw32.static-g++ -DRELEASE -DCROSS_LINUX -DGIT_VERSION -DLOCALIZE -ffast-math -Os  -Wall -Wextra   -Werror -std=c++11 -MMD  -DLUA -I../src -Wno-unused-variable -Wno-sign-compare -Wno-unknown-pragmas -Wno-parentheses -c iuse_test.cpp -o obj/iuse_test.o
objwin/cursesport.o:cursesport.cpp:(.text+0xbc1): undefined reference to `handle_additional_window_clear(WINDOW*)'
collect2: error: ld returned 1 exit status
make: *** [cataclysm.exe] Error 1
make: *** Waiting for unfinished jobs....
@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2016

Nope, that removes any pressure whatsoever from keeping the tests building, then they bitrot and become useless.

They aren't trivial to run on any supported platform. They're quite bittrotten already.
Linux x64 here, trying to run cata_tests just segfaults instantly.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jun 26, 2016

So fix it or at least report it with some supporting information, I'd rather start dropping platforms than drop the tests. Seriously, if we don't get some momentum behind unit tests, this project WILL implode under its own weight.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Jun 26, 2016

Looks like it's fixed as of build 5136, and I'm not entirely sure why. I'm guessing narc did something on his end.

@narc0tiq

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2016

I'm guessing narc did something on his end.

Nope, I've done nothing.

@narc0tiq

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2016

Weird, I definitely recall there being a make clean in the mainline-build script. Apparently, that went missing at some point. It's why 5135 wasn't marked as fixing the issue -- for some reason, make decided not to rebuild tests/visitable_remove.cpp until the later build.

I've put the make clean back in where it should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.