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

New build system #50

Merged
merged 53 commits into from
Mar 27, 2018
Merged

New build system #50

merged 53 commits into from
Mar 27, 2018

Conversation

janstary
Copy link
Contributor

This gets rid of GNU autotools and introduces a simple, hand-written ./configure
that uses a set of trivial have-*.c programs to test whether e.g. err() exists,
or whether socket() needs -lsocket, etc. See the README.

mcd500 and others added 30 commits March 22, 2018 11:30
Tweaking files on a github and in a source tar file

Signed-off-by: Akira Tsukamoto <[email protected]>
rtpdump.c never user the declared fmod()
and nothing uses the declared infinity()
This makes './compile; make' build rtptools without GNU
and without autoregenerating anything. The ./configure
script is a simple, hand-written, standalone shell script
accompanied with a set of trivial have-*.c check programs
and compat-*.c implementations.

Currently, we only check for err() and strtonum(),
and whether struct msghdr has msg_control; this is
a proof of concept - we need to go through the old
configure.ac and Makefile.am and trhrough all the sources
to make sure nothing is left behind.

The idea of replacing the GNU autohell with this
comes from http://mandoc.bsd.lv/cgi-bin/cvsweb/INSTALL
THese are the remaining system functions etc
that we need to check for with have-*.c and set in config.h
remove and ignore regenerated files
@janstary
Copy link
Contributor Author

janstary commented Mar 24, 2018

This is the shell code that runs the test in the proposed ./configure:

singletest() {
        cat 1>&3 << __HEREDOC__
${1}${3}: testing...
${COMP} -o have-${1} have-${1}.c ${3}
__HEREDOC__

        if ${COMP} -o "have-${1}" "${SOURCEDIR}/have-${1}.c" ${3} 1>&3 2>&3
        then
                echo "${1}${3}: ${CC} succeeded" 1>&3
        else
                echo "${1}${3}: ${CC} failed with $?" 1>&3
                echo 1>&3
                return 1
        fi

        if ./have-${1} 1>&3 2>&3; then
                echo "${1}${3}: yes" 1>&2
                echo "${1}${3}: yes" 1>&3
                echo 1>&3
                eval HAVE_${2}=1
                rm "have-${1}"
                return 0
        else
                echo "${1}${3}: execution failed with $?" 1>&3
                echo 1>&3
                rm "have-${1}"
                return 1
        fi
}

@mcd500
Copy link
Contributor

mcd500 commented Mar 25, 2018

There are several topics need to be discussed for this pull request.

Having a script for generating Makefile for different system will make everybody easier to run the rtptools on different platform.
The rtptools is used as the reference code for who would like to start write program based on RFC3650 and rtptools historically focusing to be on multiple platform was strong value for the purpose.

The release 1.22 is using autotols, just because I wanted to minimize the breakage and I was trying to be conservative when fixing the build adopting to circumstance from the older releases.
In the 1.88 era, most of them were 32bit systems. I personally do not have any strong preference what build system to use for it.

When @janstary was referring the new build system, I thought he was going to use cmake, waf or etc which already exists. Then I saw it was brand new configure script written from scratch. :)
(We may end of making a new build system spawned from rtptools project)

As @janakj mentioned, considering the size of rtptools, the auotools is overkill.
However, I strongly prefer to have cross-compositing capability.
I just checked it after removing the clang detection in configure.ac the following works fine on 1.22 release.

$ ./configure --host=arm-linux-gnueabihf
$ make clean
$ make
$ file rtptrans
rtptrans: ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 2.6.32, BuildID[sha1]=cbf96a80fd1bf52487f6a5ef1d33d177142beaf6, not stripped

With
$ ./configure --host=aarch64-linux-gnu
then
$ file rtptransrtptrans: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, for GNU/Linux 3.7.0, BuildID[sha1]=016631761b0156d6ac548e8dad629755b8b05e83, not stripped

It looks like just changing the compiler in the CC which might not be so difficult to adapt on new configure script.

Other topics here is that license of autotools which is GPL.
The post here may related to the autotool.
#51

There is no problem in point of license that autotools is being GPL for OpenBSD.
Using GPL programs do not effect of what license to be used for generated programs.
For example, the source which is not GPL and using gcc to compile and the binary would not be GPL is perfectly fine. The GPL do not enforce other program to be GPLunless it contains some portion of GPL sources. Also the none GPL binary being executing while there are GPL binaries but having the context switch, e.g, in other process, between kernal and userlan, also do not have to be GPL.

The autotools generates configure from configure.am and Makefile.am.
And the generated configure will generate Makefile.
At this point configure uses aclocal.m4, missing, install-sh, config.sub, config.guess, compile, depcomp, which are all GPL programs.
However, all configure, configure.am, Makefile.am, Makefile are not GPL and do not have to be GPL from legal point of view of the GPL license.

I understand that FreeBSD and OpenBSD project are going away from GPL programs, and replacing glibc, gcc and others.
Then they may not like having GPL programs, such as, aclocal.m4, missing, install-sh, config.sub, config.guess, compile, depcomp’, in the tar file of rtptools.
Then we have to remove aclocal.m4, missing, install-sh, config.sub, config.guess, compile, depcomp’ from the rtptools package.

@janstary
Copy link
Contributor Author

The build system should be a thin layer upon the actual code.
What we have now is the exact opposite, hence this proposal.

The build system introduced here is not new and is not my invention;
it comes directly from http://mandoc.bsd.lv/ (the extremely portable
manpage formatter in *BSD, MacOS, and a couple of Linux distros).
It is not my whim, it is well setablished.

Regarding cross-compiling: it is the compiler's job to produce code for the given platform.
The build system's (i.e. ./configure's) role in this is to pass the option. I can add that,
but I don't think it's worth it at all: do we need to cross compile? Do we have a platform
on which we would like to run, but don't have a native compiler for? Currently, we provide
the source tarball (of course), a Windows binary (Intel only), and a RPM (Intel only).
I don't think cross-compiling is an argument against this.

Regarding license: what I mean by acceptable license in #51
is the change from the Columbia license (1.21) to BSD (1.22).
GPL itself is not a problem, but of course BSD is preferable.

In autotools (as I understand it), autoconf produces ./configure from ./configure.ac,
automake produces Makefile.in from Makefile.am,
and then ./configure produces Makefile from Makefile.in, with the help of
aclocal.m4, missing, install-sh, config.sub, config.guess, compile, depcomp.

I find that quite overingeneered. In the proposed system, you have a static Makefile
and a static ./configure script which produces Makefile.local and config.h
with the help of have-*.c.

@janakj
Copy link
Contributor

janakj commented Mar 25, 2018

Regarding cross-compilation: I don't know if you need it. It was my impression that you care about supporting multiple operating systems and platforms. If that's the case, providing support for cross-compilation would make sense. It's not that difficult to do in a small project like rtptools, one just needs to keep in mind that programs may need to be compiled and run in the configuration phase.

In my opinion, having cross-compilation support is useful, because it helps build the software for various embedded platforms, e.g., those based ARM SoCs or Android. I hear those are popular these days.

@janstary
Copy link
Contributor Author

Supporting multiple operating systems and platforms is definitely the goal,
but that's not the same as cross-compiling. Do we know of a OS/platform
where we would like to run, but can't compile natively (and need to cross-compile)?
Running RTP tools on e.g. Android seems a bit far-fetched.

@janstary
Copy link
Contributor Author

So what happens now? Do we want this?
Do we want to stay with GNU autotools?
Who's gonna make that decision?

README.md Outdated
are now reasonably close to POSIX and do not need arcane shell magic
any longer. If your system does need such magic, consider upgrading
to reasonably modern POSIX-compliant tools rather than asking for
autoconf-style workarounds.
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't like autotools and that's fine. But is this text really necessary? README.md is one of the first files a new user encounters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no objections to removing this. I felt it appropriate to inform the user
that the ./configure script is not what he might be expecting from elsewhere.
I will remove it.

rtpdump.o: rtpdump.c rtp.h sysdep.h vat.h rtpdump.h ansi.h
rtpplay.o: rtpplay.c sysdep.h notify.h rtp.h rtpdump.h multimer.h ansi.h
rtpsend.o: rtpsend.c notify.h rtp.h sysdep.h multimer.h ansi.h
rtptrans.o: rtptrans.c rtp.h sysdep.h rtpdump.h ansi.h notify.h multimer.h vat.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that the list of compile-time dependencies is fixed? Are developers expected to keep this file synchronized with the source code?

Wouldn't it be better and more robust to let the compiler (gcc) generate the list of dependencies automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This list is generated by mkdep. It has not changed in a decade,
and can be redone with make depend

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I think make depend is fine (as long as there is one command to update the dependency list).

README.md Outdated

### build

Once the source is configured as above, run `make` to build SoX.
Copy link
Contributor

Choose a reason for hiding this comment

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

SoX -> rtptools?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copypasta - fixed, thanks.

main(void)
{
struct hostent* host;
host = gethostbyname("columbia.edu");
Copy link
Contributor

@janakj janakj Mar 25, 2018

Choose a reason for hiding this comment

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

Is it a good idea to invoke a function that internally performs a DNS request? Would it not be possible to test for the presence of gethostbyname without invoking it (and waiting for the DNS response to arrive or timeout)?

If gethostbyname needs to be invoked, perhaps the test could pass an IPv4 address to it, e.g., "127.0.0.1", or a name that is unlikely to trigger name resolution such as "localhost".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, localhost is probably better. Thanks.

@mcd500
Copy link
Contributor

mcd500 commented Mar 27, 2018

Regarding license: what I mean by acceptable license in #51
is the change from the Columbia license (1.21) to BSD (1.22).
GPL itself is not a problem, but of course BSD is preferable.

Thanks for clarification of your intention regarding the license and the
#51

I think we could close of having consensus on the license topics.

@mcd500
Copy link
Contributor

mcd500 commented Mar 27, 2018

For the cross-compilation, almost nobody likes cross-compilation for the developing products.
The most of the reasons of using cross-compilation are;
() The target cpu is so slow for compiling and building on PC will dramatically reduce the time of CI loop. Most of the time for product development, time is money, The PC on Intel/AMD is the fastest machine to build at the moment on most cases.
(
) The board with target cpu is not able to even build by variety of limitations, often have too small memory size which result having OOMKiller on the target board, the number of boards are only produced limited numbers and the developers do not have his/her own board to build on it to check before the commits, many boards of engineering samples cost more than 500,000,000USD.

We have to adopt world if we do not have the alternative and we can not fix the world.
The cross-complier are mostly developed after the native compiler are working.

I feel it is OK to add cross-compiling feature after merging the new configure script.
Adding the make rpm later too.

@janstary
Copy link
Contributor Author

janstary commented Mar 27, 2018

I don't think the cross-compiling arguments above apply to our case.

On my Beagle Bone Black running OpenBSD 6.3/armv7,
./configure ; make takes about 12 seconds (with this new system).
That's hardly "so slow" we would need to cross-compile.

The arm* boards are pretty well supported nowadays,
at least they run an OS and a compiler (mostly various linuxes and BSDs).

I don't think cross-compilation is worth the added complexity.
If someone has an embedded board where he needs to run rtptools
but cannot compile natively, I will be glad to hear about his cool application and add it.

I agree that both this and the rpm (and possibly deb) packages
can be done after this is merged in.

So, is there anything stopping us from merging?

@janstary
Copy link
Contributor Author

janstary commented Mar 27, 2018

BTW, the old aclocal ; automake; autoconf; ./configure; make clean all takes about 76s
(not counting the time to build autoconf, automake, pkg-config, help2man and xz),
out of which the actual make clean all is about 19s; that's about 400% overhead.

With this new system, ./configure ; make clean all is about 12s,
out of which the actual make clean all is about 11s; that's about 10% overhead.

(Note how the new ./configure && make clean all is faster, including ./configure,
than make clean all itself in the old system. Same make(1), same cc(1),
but a different Makefile.)

@mcd500
Copy link
Contributor

mcd500 commented Mar 27, 2018

I don't think the cross-compiling arguments above apply to our case.

How about the other reason I mentioned?
I see only about comparing the compile speed in your comment against the Beagle Bone Black which is super fast already but how about other reasons that huge number of engineers are cross-compiling everyday.

Please try to build natively on any of the HP/Lexmark printers.
(Most of the time they even do not have compiler in the rootfs even during the development)

Software engineering is normally a team work.
Instead of making impression to others that anything you have never experienced is ignorable,
it is always good to show that you are trying to support other persons perspective.

@janstary
Copy link
Contributor Author

janstary commented Mar 27, 2018

We already agreed to merge this. Can you please merge this?

If you need to run rtptools on a Lexmark printer,
or some othe platform that needs cross-compiling,
it's easy enough to add after this is in.

@mcd500
Copy link
Contributor

mcd500 commented Mar 27, 2018

Ok :)
On this thread, all what matter is the configure script.

If you need to run rtptools on a Lexmark printer,
or some othe platform that needs cross-compiling,

Lets not go on this road any further.

@mcd500 mcd500 merged commit 8962acc into irtlab:master Mar 27, 2018
@janstary janstary deleted the build branch March 27, 2018 15:47
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.

4 participants