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

Addition of a build system generator #9

Closed
elfring opened this issue Aug 17, 2015 · 36 comments
Closed

Addition of a build system generator #9

elfring opened this issue Aug 17, 2015 · 36 comments

Comments

@elfring
Copy link
Contributor

elfring commented Aug 17, 2015

I suggest to reuse a higher level build system than your current small make file so that powerful checks for software features will become easier.

@johnkerl
Copy link
Owner

This is a really great idea. The current is definitely simple and quick for a dev box; since the release announcement with more people building on more platforms, it'll quickly become essential. For configuring install dir, if nothing else.

@johnkerl
Copy link
Owner

If you've used cmake or autotools before (I have not) and have any sage advice, bring it on! :)

@elfring
Copy link
Contributor Author

elfring commented Aug 18, 2015

@bokmann bokmann mentioned this issue Aug 19, 2015
@johnkerl
Copy link
Owner

Marking as wishlist for now, just relative to other things more pressing -- namely, RFC-CSV and packaging. In the medium term, though, this will be a should-do rather than a wishlist item.

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Aug 29, 2015

I've started an autoconf/automake based build structure in https://github.com/0-wiz-0/miller
mlr itself builds; I haven't tested it.
There is still lots of stuff to do:
tests
doc/
custom targets
distribution

To try it out:
install autoconf, automake, libtool
checkout my fork https://github.com/0-wiz-0/miller
autoreconf -fiv
./configure
make

@elfring
Copy link
Contributor Author

elfring commented Aug 29, 2015

checkout my fork

0-wiz-0 added a commit to 0-wiz-0/miller that referenced this issue Aug 29, 2015
Use AC_CONFIG_AUX_DIR as suggested by elfring in
johnkerl#9
@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Aug 29, 2015

elfring, thanks for the suggestions. I did add an AC_CONFIG_AUX_DIR call.
I also read all automake options but didn't find any I would add to the AM_INIT_AUTOMAKE call.
Let me know if I overlooked anything.

@elfring
Copy link
Contributor Author

elfring commented Aug 29, 2015

Let me know if I overlooked anything.

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Aug 29, 2015

foreign is in the top level Makefile.am; there's no point in adding it to AM_INIT_AUTOMAKE, since it doesn't do anything in the other Makefiles.
Wall is only interesting for people developing the Makefile.ams. You can add it from the command line. Again, there is no point in other Makefiles than the top-level one.
I've removed the AM_PROG_CC_C_O, thanks for pointing out it's on the way out!

@elfring
Copy link
Contributor Author

elfring commented Aug 29, 2015

…; there's no point in adding it to …

I guess that there is a trade-off involved. How many settings should be moved from the file "Makefile.am" to the primary script "configure.ac"?

@johnkerl johnkerl removed the low-pri label Aug 30, 2015
@johnkerl
Copy link
Owner

Nice!

@johnkerl
Copy link
Owner

johnkerl commented Sep 4, 2015

This is cool. There's no push request yet but this is something to work with. I'm currently doing what I consider higher-pri work on file formats & read performance. But, this is important. Some of the feedback I've found off Github (by searching for it) is that my failure to provide a standard ./configure && make experience out of the box was off-putting for some.

@johnkerl
Copy link
Owner

johnkerl commented Sep 4, 2015

... by which I mean, if someone else finishes the last bits, great, else I will. So we'll get there either way.

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Sep 4, 2015

I've worked on it some more.
All the unit tests are now run with 'make check'.
'make distcheck' succeeds, i.e. 'make dist' generates tarballs that can be extracted, configured, built, and finish 'make check' successfully.
The man page is now installed (but only generated on the system generating the tarball, so not everyone needs to have asciidoc installed).

Can you please take a closer look and recommend which files should be distributed that currently aren't?
In particular, in doc (but consider that not everyone should have to install poki).
but also for data (but see #61).

I know that there is one more test, test/run, that should be added.

One issue I found is that the lemon executable needs its input files in ".". Perhaps it can be extended to support being called with an input file in ${srcdir} while being in ${objdir}, i.e. being called with "lemon ../../c/dsls/inputfile"?

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Sep 4, 2015

I've worked on test/run a bit. I have the problem that this script has paths in the expected output. Can you please modify it so it doesn't, e.g. by calling basename on executable and input(s) (just for the debugging output)?
Otherwise out-of-src-tree builds will never run tests successfully, and 'make distcheck' uses these for testing.
Except for that it seems easy to add this script as well, I've done it locally but not pushed yet (because of -- see above).

@johnkerl
Copy link
Owner

johnkerl commented Sep 4, 2015

Out-of-dir builds, very good idea in any case.

OK I think I'll fork your fork ... get it where I want it to be ... then make you a pull request. Then you can make me a pull request to master -- sound OK?

@elfring
Copy link
Contributor Author

elfring commented Sep 4, 2015

I think I'll fork your fork …

Are you aware of better Git development techniques?

@johnkerl
Copy link
Owner

johnkerl commented Sep 4, 2015

Apparently not :^/. I'll take a read.

@elfring
Copy link
Contributor Author

elfring commented Sep 5, 2015

@0-wiz-0, @johnkerl: How do you think about to improve the software development collaboration by the reuse of topic branches?

@johnkerl
Copy link
Owner

johnkerl commented Sep 5, 2015

sure, forks and branches are morally the same, either will work. whatever's easiest. 99% of the time i prefer unbranched iteration in master, but here either forks or branches will be called for since the changes could temporarily break master.

@elfring
Copy link
Contributor Author

elfring commented Sep 5, 2015

I find that a topic branch should usually get an other name than "master".
To which branch would you like to contribute the next updates here?

@jungle-boogie
Copy link
Contributor

Hi @johnkerl, yes, develop on trunk/master!

http://www.tedunangst.com/flak/post/branchless-development

@johnkerl
Copy link
Owner

johnkerl commented Sep 9, 2015

All -- thanks for the software-engineering discussion. Over my career, on the whole I've become a strong advocate for developing in master almost all the time, with frequent merges. This is good for small projects; it's a make-or-break for large projects/companies. That said, every once in a while a short-lived branch is useful -- and the implementation details don't matter too much -- which is the case here since autoconfigure testing will break master, and/or just confuse people, until it's complete.

@johnkerl
Copy link
Owner

Pushed out-of-directory test/run to https://github.com/0-wiz-0/miller. Ran as:

  • ./test/run
  • cd ../; ./c/test/run
  • cd; ~/tmp/wiz/miller/c/test/run

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Sep 10, 2015

Thanks!
But isn't that only half the solution? When I run this from "c/build" with "../test/run", I have the following diff in my source tree afterwards:

-mlr having-fields --at-least a,b,i,x,y /Users/kerl/tmp/wiz/miller/c/test/input/abixy
+mlr having-fields --at-least a,b,i,x,y ../test/input/abixy

In other words, the path to the input files also needs sanitizing, I think.

@johnkerl
Copy link
Owner

That case was included in the diff, using "diff -I '^mlr '" in test/run. It
works for me from all sorts of directories, on OSX. I'll investigate more,
late this evening.

On Thu, Sep 10, 2015 at 9:33 AM, Thomas Klausner [email protected]
wrote:

Thanks!
But isn't that only half the solution? When I run this from "c/build" with
"../test/run", I have the following diff in my source tree afterwards:

-mlr having-fields --at-least a,b,i,x,y /Users/kerl/tmp/wiz/miller/c/test/input/abixy
+mlr having-fields --at-least a,b,i,x,y ../test/input/abixy

In other words, the path to the input files also needs sanitizing, I think.


Reply to this email directly or view it on GitHub
#9 (comment).

John Kerl
[email protected]
http://johnkerl.org
https://github.com/johnkerl http://johnkerl.github.com

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Sep 10, 2015

Ah, I had overlooked that. Thanks.
I have two issues:

  1. When run in-tree, it overwrites c/test/output/out
  2. When run out-of-tree during 'make distcheck' the source tree is read-only, which makes 'run' fail with:
mkdir: ../../../../c/test/output: Permission denied

@johnkerl
Copy link
Owner

ok. test/output/out should not be checked into source control; it should
be created by test/run. this was my oversight. i'll take a look at make distcheck.

thank you!!!! :)

On Thu, Sep 10, 2015 at 10:17 AM, Thomas Klausner [email protected]
wrote:

Ah, I had overlooked that. Thanks.
I have two issues:

  1. When run in-tree, it overwrites c/test/output/out
  2. When run out-of-tree during 'make distcheck' the source tree is
    read-only, which makes 'run' fail with:

mkdir: ../../../../c/test/output: Permission denied


Reply to this email directly or view it on GitHub
#9 (comment).

John Kerl
[email protected]
http://johnkerl.org
https://github.com/johnkerl http://johnkerl.github.com

@johnkerl
Copy link
Owner

Wiz, thanks very much for your ongoing expert help!! I'm new to autoconf, so just to be sure I know the steps:

  • autoreconf -fiv
  • Then the resulting ./configure gets checked into source control so not all end-users need to have autoconf, automake, and libtool installed
  • ./configure run by end-user
  • Interesting dev targets are make, make check, and make distcheck -- ?
  • Interesting end-user targets are make, make check, and make install -- ?
  • Are there important other make targets and if so where in the workflow do they belong?

@johnkerl
Copy link
Owner

I'll be reading through automake.pdf on today's flight as well.

@johnkerl
Copy link
Owner

Another test case for myself is VPATH build with read-only source tree

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Sep 11, 2015

The steps are good, and you mentioned all the main targets.
There is a tags target as well, I think.
You can give debugging etc. flags on the configure command line like this:

./configure CFLAGS="-g -O0"

The configure script itself is usually not checked in. If you did, you would have to check in many more files as well (all Makefile.in and the support scripts like libtool, config.guess...).
autoconf is more aimed at a distfile workflow: someone with autotools creates a distfile (make distcheck) which contains all the necessary files. The user of a distfile does not need autotools or libtool installed.
Hope this helps. Feel free to ask more!

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Sep 11, 2015

Oh, and you probably are interested in the "--disable-shared" configure argument -- that should give you a static executable without dependencies, that you can easily copy to other machines.

@johnkerl
Copy link
Owner

ALso the pdm and fdm executables in c/dsls -- I should turn these into unit-test code, or abandon them.

@johnkerl
Copy link
Owner

The merge is in. To do:

  • Have the Travis build go through autoconfig
  • User-level docs
  • Clearly delineate in the docs what needs to be done at package-build time vs. at package-install time
  • Try to reduce the number of dependencies for package install: manpage/mlrdoc/etc. should be artifacts at that point in time with no need for xmllint, poki, etc.

Thank you @0-wiz-0 !!!! :D

@johnkerl johnkerl mentioned this issue Sep 22, 2015
@johnkerl johnkerl removed the active label Sep 24, 2015
@johnkerl
Copy link
Owner

This is complete with docs at http://johnkerl.org/miller/doc/build.html.

There is some separate manpage-related work tracked on #56.

If there are any subsequent build errors, feel free to open a new issue as always. And thanks again @0-wiz-0. :)

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

No branches or pull requests

4 participants