Use autotools build system #14

Merged
merged 13 commits into from Jun 1, 2016

Conversation

Projects
None yet
4 participants
Collaborator

zyga commented May 30, 2016

This patch discards the hand-crafted build system in favour of autotools.
This change makes it easier to build the source code and to begin adding
conditional features.

The seccomp.[ch] file was renamed to seccomp_utils.[ch] in order not to
clash with the system-wide seccomp.h. This is required because of how
autotools pass the -I. flag by default.

Unit tests were converted to autotool standards.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

zyga added some commits May 30, 2016

Use autotools build system
This patch discards the hand-crafted build system in favour of autotools.
This change makes it easier to build the source code and to begin adding
conditional features.

The seccomp.[ch] file was renamed to seccomp_utils.[ch] in order not to
clash with the system-wide seccomp.h. This is required because of how
autotools pass the -I. flag by default.

Unit tests were converted to autotool standards.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Drop lib/* from snap-run.install
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@@ -0,0 +1,11 @@
+bin_PROGRAMS = snap-run
+snap_run_SOURCES = main.c seccomp_utils.c utils.c
+snap_run_CFLAGS = \
@mvo5

mvo5 May 31, 2016

Contributor

Hm, no more make fmt and make syntax-check ? And will make test work as before? It is also dropping the udev releated stuff unless I miss something.

@zyga

zyga May 31, 2016

Collaborator

make fmt is indeed gone, I'll bring it back. Make test is the autotools-std make check instead.
I see I don't install the udev helper, thanks for spotting that!

@zyga

zyga May 31, 2016

Collaborator

check syntax restored (for shell scripts), looking at .c and installing the small udev helper

@zyga

zyga May 31, 2016

Collaborator

check syntax restored (for C files), looking at the last thing

@zyga

zyga May 31, 2016

Collaborator

Udev rules restored, testing with local packages now

zyga added some commits May 31, 2016

Run shellcheck on make check
Signed-off-by: Zygmunt Krynicki <zkrynicki@gmail.com>
Restore C style checks
Signed-off-by: Zygmunt Krynicki <zkrynicki@gmail.com>
Install udev rules/support files
Signed-off-by: Zygmunt Krynicki <zkrynicki@gmail.com>
Add a comment about check-syntax
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Add newline to the end of the file
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Collaborator

zyga commented Jun 1, 2016

make fmt restored

+AC_FUNC_ERROR_AT_LINE
+AC_FUNC_FORK
+AC_FUNC_STRNLEN
+AC_CHECK_FUNCS([mkdir regcomp setenv strdup strerror])
@tyhicks

tyhicks Jun 1, 2016

Collaborator

I don't know if it is important to check all the functions used by snap-confine here (I typically don't do it in the projects that I'm involved in) but there are some missing functions in this list.

@zyga

zyga Jun 1, 2016

Collaborator

This is just autoscan, I suspect that we can just remove this whole chunk as those functions don't really differ across systems anymore.

Thanks for having a look!

Collaborator

tyhicks commented Jun 1, 2016

Have you done a comparison of the build logs before and after applying this PR to verify that compiler flags did not change for the worse?

Collaborator

zyga commented Jun 1, 2016

@tyhicks I didn't do that, that's a good idea.

debian/control
@@ -2,7 +2,7 @@ Source: ubuntu-core-launcher
Section: utils
Priority: optional
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
-Build-Depends: debhelper (>= 9), libseccomp-dev, libapparmor-dev, libudev-dev, dh-apparmor, indent, shellcheck
+Build-Depends: debhelper (>= 9), libseccomp-dev, libapparmor-dev, libudev-dev, dh-apparmor, indent, shellcheck, pkg-config
@ogra1

ogra1 Jun 1, 2016

don't you need autotools-dev and autoconf, automake build deps as well here ?

@zyga

zyga Jun 1, 2016

Collaborator

Not according to running this in sbuild. I can add it though.

@zyga

zyga Jun 1, 2016

Collaborator

Now with --autoreconf everything works. Thanks for correcting me ogra!

Collaborator

tyhicks commented Jun 1, 2016

I tried to build a debian package with sbuild and it fails:

https://paste.ubuntu.com/16897029/

Nothing is being built. This diff between builds before and after this PR is applied shows that to be the case:

https://paste.ubuntu.com/16897178/

Collaborator

zyga commented Jun 1, 2016

Thanks for testing this. Let me try to reproduce that.

Generate autotools build system at package build time
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Collaborator

zyga commented Jun 1, 2016

Ah, indeed, I just had a dirty tree with ./configure around. Fixed now.

Ensure that -Wall and -Werror are in CFLAGS
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Collaborator

zyga commented Jun 1, 2016

Merging based on review by @tyhicks and ogra on IRC

@zyga zyga merged commit 51cb2b7 into master Jun 1, 2016

@zyga zyga deleted the autotools branch Jun 2, 2016

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