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

Make Meson work with 0.8.x #1894

Merged
merged 4 commits into from Aug 21, 2017

Conversation

Projects
None yet
2 participants
@ximion
Contributor

ximion commented Aug 17, 2017

The Meson configuration is modularized as well now - there are some improvements that can be done (the files are a bit verbose), and maybe there are even some issues left, but I will find most of those when packaging the new version for Debian.

I also enabled CI for the builds.

@ximion

This comment has been minimized.

Show comment
Hide comment
@ximion

ximion Aug 18, 2017

Contributor

With LDC 1.3.0 I get

9495 Illegal instruction     (core dumped) ./core/vibe-test_core

This seems to be an LDC bug...

Contributor

ximion commented Aug 18, 2017

With LDC 1.3.0 I get

9495 Illegal instruction     (core dumped) ./core/vibe-test_core

This seems to be an LDC bug...

@ximion

This comment has been minimized.

Show comment
Hide comment
@ximion

ximion Aug 18, 2017

Contributor

@s-ludwig You would need to take a look at the test_version script for this to work. Library versioning on Linux doesn't allow letters in the version, so a build will fail if you attempted that. I also set a project-wide version now, which has the same restrictions.
If we want to use the full version in Meson somewhere (we currently don't), we compose if from the given version and a suffix, that can be set in the main file as well.

I don't know what you like here, so I just disabled this for now to let the CI continue its job.
It also appears we hit a couple of LDC bugs here, which don't appear in DMD... I guess those will have to be resolved as well (see the linked bug report above).
I would really like to have this in the next release, so I can get the Debian package into the testing suite properly.

Contributor

ximion commented Aug 18, 2017

@s-ludwig You would need to take a look at the test_version script for this to work. Library versioning on Linux doesn't allow letters in the version, so a build will fail if you attempted that. I also set a project-wide version now, which has the same restrictions.
If we want to use the full version in Meson somewhere (we currently don't), we compose if from the given version and a suffix, that can be set in the main file as well.

I don't know what you like here, so I just disabled this for now to let the CI continue its job.
It also appears we hit a couple of LDC bugs here, which don't appear in DMD... I guess those will have to be resolved as well (see the linked bug report above).
I would really like to have this in the next release, so I can get the Debian package into the testing suite properly.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Aug 19, 2017

Member

So if we would set the first three digits of the full version as the meson project version and set the rest as the suffix, would that work correctly for upgrades (i.e. would it detect that 1.0.0 + "~beta.1" is a lower version than "~rc.1", or can we achieve that with other means)?

I wouldn't mind if the LDC+libevent meson tests were just skipped for now, until the crash gets fixed in LDC. The future direction will be the vibe-core/eventcore configuration anyway, so it wouldn't be a big loss at this point.

Member

s-ludwig commented Aug 19, 2017

So if we would set the first three digits of the full version as the meson project version and set the rest as the suffix, would that work correctly for upgrades (i.e. would it detect that 1.0.0 + "~beta.1" is a lower version than "~rc.1", or can we achieve that with other means)?

I wouldn't mind if the LDC+libevent meson tests were just skipped for now, until the crash gets fixed in LDC. The future direction will be the vibe-core/eventcore configuration anyway, so it wouldn't be a big loss at this point.

@ximion

This comment has been minimized.

Show comment
Hide comment
@ximion

ximion Aug 19, 2017

Contributor

@s-ludwig The tilde (~) lowers the version number, so 0.1.2 >> 0.1.2~rc1. This is true in Debian and AFAIR RPM-based distros as well, since the dash is reserved for the distribution revision. We actually rewrite the upstream version name to that format prior to comparing versions, as 0.1.2 << 0.1.2-rc1, so we get notified properly when a new upstream release is there.
I think I used this in the Meson file just out of habit.

I wouldn't mind if the LDC+libevent meson tests were just skipped for now, until the crash gets fixed in LDC. The future direction will be the vibe-core/eventcore configuration anyway, so it wouldn't be a big loss at this point.

I can skip them - but does that mean I should switch the Meson configuration to use vibe-core/eventcore? I would like to have it use the recommended codepath for most projects, and the libevent one seemed to be that for a while.

I could also cheat and have the libevent+LDC CI job not run the tests until the bug is fixed...

Contributor

ximion commented Aug 19, 2017

@s-ludwig The tilde (~) lowers the version number, so 0.1.2 >> 0.1.2~rc1. This is true in Debian and AFAIR RPM-based distros as well, since the dash is reserved for the distribution revision. We actually rewrite the upstream version name to that format prior to comparing versions, as 0.1.2 << 0.1.2-rc1, so we get notified properly when a new upstream release is there.
I think I used this in the Meson file just out of habit.

I wouldn't mind if the LDC+libevent meson tests were just skipped for now, until the crash gets fixed in LDC. The future direction will be the vibe-core/eventcore configuration anyway, so it wouldn't be a big loss at this point.

I can skip them - but does that mean I should switch the Meson configuration to use vibe-core/eventcore? I would like to have it use the recommended codepath for most projects, and the libevent one seemed to be that for a while.

I could also cheat and have the libevent+LDC CI job not run the tests until the bug is fixed...

ximion added some commits Aug 18, 2017

ci: Do not test versions
This needs an update, as the suffix (~rc, etc.) isn't allowed for
library versioning, so we need to deal with that separately.
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Aug 20, 2017

Member

I can skip them - but does that mean I should switch the Meson configuration to use vibe-core/eventcore? I would like to have it use the recommended codepath for most projects, and the libevent one seemed to be that for a while.

I could also cheat and have the libevent+LDC CI job not run the tests until the bug is fixed...

Oh, no, I misinterpreted the results as the Meson build running on both, libevent and vibe-core configurations (didn't remember the constraint in travis.sh). My comment doesn't really make sense then, as the tests may likely fail to build with vibe-core, too.

Just not running the tests would be the cheat that I had in mind then.

Member

s-ludwig commented Aug 20, 2017

I can skip them - but does that mean I should switch the Meson configuration to use vibe-core/eventcore? I would like to have it use the recommended codepath for most projects, and the libevent one seemed to be that for a while.

I could also cheat and have the libevent+LDC CI job not run the tests until the bug is fixed...

Oh, no, I misinterpreted the results as the Meson build running on both, libevent and vibe-core configurations (didn't remember the constraint in travis.sh). My comment doesn't really make sense then, as the tests may likely fail to build with vibe-core, too.

Just not running the tests would be the cheat that I had in mind then.

@ximion

This comment has been minimized.

Show comment
Hide comment
@ximion

ximion Aug 20, 2017

Contributor

Just not running the tests would be the cheat that I had in mind then.

Applied :-) But long-term it looks like I should support vibe-core in Meson as well as libevent then :)
The CI should be happy now, with skipped tests on LDC builds (they are run with DMD, which doesn't have this bug).

Contributor

ximion commented Aug 20, 2017

Just not running the tests would be the cheat that I had in mind then.

Applied :-) But long-term it looks like I should support vibe-core in Meson as well as libevent then :)
The CI should be happy now, with skipped tests on LDC builds (they are run with DMD, which doesn't have this bug).

@ximion

This comment has been minimized.

Show comment
Hide comment
@ximion

ximion Aug 20, 2017

Contributor

On DMD 2.072.2 the vibe-test_stream fails:

5/13 vibe-test_stream                        FAIL     0.29 s
--- command ---
/home/travis/build/ximion/vibe.d/build/stream/vibe-test_stream
--- stderr ---
/home/travis/build/ximion/vibe.d/build/stream/vibe-test_stream(_D4core7runtime18runModuleUnitTestsUZ19unittestSegvHandlerUNbiPS4core3sys5posix6signal9siginfo_tPvZv+0x38)[0x5618ac]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x10330)[0x7ff0cd264330]
-------

All other DMD versions are fine. Smells like another compiler bug to me, I will make Travis skip the tests on that particular DMD version as well.

Contributor

ximion commented Aug 20, 2017

On DMD 2.072.2 the vibe-test_stream fails:

5/13 vibe-test_stream                        FAIL     0.29 s
--- command ---
/home/travis/build/ximion/vibe.d/build/stream/vibe-test_stream
--- stderr ---
/home/travis/build/ximion/vibe.d/build/stream/vibe-test_stream(_D4core7runtime18runModuleUnitTestsUZ19unittestSegvHandlerUNbiPS4core3sys5posix6signal9siginfo_tPvZv+0x38)[0x5618ac]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x10330)[0x7ff0cd264330]
-------

All other DMD versions are fine. Smells like another compiler bug to me, I will make Travis skip the tests on that particular DMD version as well.

meson: Skip tests when compiling with broken LDC/DMD
This works around LDC bug
ldc-developers/ldc#2280 for now,
and skips compilation with a broken DMD version.
@ximion

This comment has been minimized.

Show comment
Hide comment
@ximion

ximion Aug 20, 2017

Contributor

And now the CI is happy :-)

Contributor

ximion commented Aug 20, 2017

And now the CI is happy :-)

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Aug 21, 2017

Member

Okay, I'll merge and play around with the version test/tag scripts!

Member

s-ludwig commented Aug 21, 2017

Okay, I'll merge and play around with the version test/tag scripts!

@s-ludwig s-ludwig merged commit 0152cf4 into vibe-d:master Aug 21, 2017

4 checks passed

codecov/patch Coverage not affected when comparing ef0c48a...c684fa9
Details
codecov/project 44.912% (+42.747%) compared to ef0c48a
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ximion

This comment has been minimized.

Show comment
Hide comment
@ximion

ximion Aug 21, 2017

Contributor

Thank you! :-)

Contributor

ximion commented Aug 21, 2017

Thank you! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment