Skip to content

Non-recursive automake #128

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

Merged
merged 7 commits into from
Oct 16, 2016
Merged

Non-recursive automake #128

merged 7 commits into from
Oct 16, 2016

Conversation

DrMcCoy
Copy link
Member

@DrMcCoy DrMcCoy commented Sep 23, 2016

Analoguously to the non-recursive PR in xoreos-tools, this changeset adds non-recursive automake support to xoreos.

The gitstamp hackery has to be redone for various reasons with a non-recursive setup. To remove the noise during the actual non-recursive change, this changeset front-loads a few related changes.

Likewise, the Automake -> CMake bridge has to be slightly redone. This is done in a separate commits.

No copyright header fixes here, though. :P

Comments welcome! :)

@DrMcCoy DrMcCoy force-pushed the nonrecursive branch 3 times, most recently from 254cbe6 to 04a4ed8 Compare September 25, 2016 20:03
@berenm
Copy link
Contributor

berenm commented Sep 28, 2016

The CMake part looks OK to me, I am amazed that this horrible parser still does its job.

BTW I have been working on an appveyor.com integration so that Windows builds can also be automatically tested.

@DrMcCoy
Copy link
Member Author

DrMcCoy commented Sep 28, 2016

I am amazed that this horrible parser still does its job.

As long as it works, I'm pretty happy with it. :P

I have been working on an appveyor.com integration so that Windows builds can also be automatically tested

Oh, that would be very nice, yes. :)

Some checks with older compiler would be useful too, if in any way possible. The Windows and Mac OS cross-compilers I have set up here locally are relatively old gcc versions (especially the Mac OS one, a gcc 4.2.1, can't be updated without breaking the Apple stuff), but I only fire them up infrequently. The last time a few days ago I found some issues with missing includes that the newer gcc can resolve on its own somehow.

I've also been thinking about enabling CMake builds in Travis (i.e. do both, autotools and CMake). I think that should be possible by setting a custom variable in env, like USE_CMAKE. One env array item would set it to true, one to false. And then the build script itself would query that and use either CMake or autotools.

In combination with the compiler setting, that should then give use a 2x2 grid of builds:

  • autotools, gcc
  • autotools, clang
  • CMake, gcc
  • CMake, clang

This would ensure that I (or a PR or something) don't accidentally break either alternative without noticing.

In addition, I've also also started on writing unit tests, using Google Test. Still a non-public WIP branch here, but so far I already found some issues with it (the commits to common/ in the last few weeks, mainly). They're not completely clean, by-the-book unit tests (I'm not going to mock out UString in other checks and stuff like that), but I figured, it's better than nothing. That'll still take a while, though.

@berenm
Copy link
Contributor

berenm commented Sep 28, 2016

I have been working on an appveyor.com integration so that Windows builds
can also be automatically tested

Oh, that would be very nice, yes. :)

To be honest, I've been on it for quite some time... dependency management on Windows is absolutely horrible. However I think I'm seeing the end of it, thankfully because Microsoft seriously improved the C99 support in their latest releases of VS, so mostly all of xoreos dependencies can be rebuilt from source.

I'll make a PR at some point. Don't expect too much testing with older MSVC versions, I don't think it's possible.

I've also been thinking about enabling CMake builds in Travis (i.e. do
both, autotools and CMake). I think that should be possible by setting
a custom variable in env, like USE_CMAKE. One env array item would set it
to true, one to false. And then the build script itself would query that
and use either CMake or autotools.

In combination with the compiler setting, that should then give use a 2x2
grid of builds:

  • autotools, gcc
  • autotools, clang
  • CMake, gcc
  • CMake, clang

I guess you could reduce the matrix by one, I don't think trying both GCC and Clang on more than one build system will be very useful.

In addition, I've also also started on writing unit tests, using Google
Test. Still a non-public WIP branch here, but so far I already found some
issues with it (the commits to common/ in the last few weeks, mainly).
They're not completely clean, by-the-book unit tests (I'm not going to mock
out UString in other checks and stuff like that), but I figured, it's
better than nothing. That'll still take a while, though.

Nice!

@DrMcCoy
Copy link
Member Author

DrMcCoy commented Sep 28, 2016

dependency management on Windows is absolutely horrible

Ugh, yes. I already had one person give up in frustration while trying to get all the dependencies for xoreos on Windows. Boost especially gave them issues, IIRC. Losing potential contributors because of that is, well, frustrating as well.

Since Microsoft offers VMs for browser testing and things like that, I still have it on my TODO list to see if I can install Visual Studio on there and maybe create ready-made download packages with the dependencies for use in Visual Studio. Or something like that. Dunno.

I'll make a PR at some point.

I'll look forward to that, then.

I guess you could reduce the matrix by one, I don't think trying both GCC and Clang on more than one build system will be very useful.

Hmm, true.

@jdm
Copy link

jdm commented Sep 28, 2016

No idea if it will help, but Servo has run the gauntlet of appveyor integration and came up with https://github.com/servo/servo/blob/master/appveyor.yml for our various dependencies.

They're now more opaque functions in the Version namespace.
Instead of recursive automake with subdirs, we're now using non-
recursive automake using (recursive) includes.

This should speed up parallel builds because these aren't blocked
across subdirectory boundaries anymore now. Dependency change
detection and therefore subsequent partial builds are also faster with
a non-recursive setup.

On the downside, the gitstamp hackery had to be reorganized, because
build order can't be predicted anymore now.

CMake building is currently still broken as well.
Now that we're doing a non-recursive automake, the automake file
parser in our CMake scripts needed to be adapted for this as well.

The changes are as follows:
- Don't look in the configure anymore, instead specify the automake
  file to parse directly
- Handle include in the automake file, by recursively parse the
  included files and paste the result into the CMake file
- Don't prepend the full path onto source paths, as source paths are
  already fully path'd now
- Remove the extra src_ at the start of target names
- Since the gitstamp hackery is now in src/version/ instead of
  gitstamp/, we need to make sure its extra automake rule doesn't
  create non-parsable CMake, so we comment it out

The result seems to work, here on GNU/Linux at least.
@DrMcCoy DrMcCoy merged commit 68c9968 into master Oct 16, 2016
@DrMcCoy DrMcCoy deleted the nonrecursive branch October 16, 2016 21:41
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.

3 participants