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

A little list of build issues #21

Closed
Earnestly opened this issue Jul 31, 2017 · 11 comments
Closed

A little list of build issues #21

Earnestly opened this issue Jul 31, 2017 · 11 comments

Comments

@Earnestly
Copy link

Earnestly commented Jul 31, 2017

A couple of small issues related to distribution and packaging. This would ideally be handled on IRC but it appears oilshell doesn't advertise a channel anywhere I could find.

Currently using the commit a6bc98e

So far this in my experience trying to build this software. I start with ./configure --help which tells me I can use --with-readline and it appears to support the --prefix mechanism.

Running the command ./configure --prefix=/usr --with-readline simply exits with a return status of 1.

When using sh -x (since the script defines itself to be /bin/sh) it seems to show the intended error indicating a failure to detect readline:

+ FLAG_prefix=/usr/local
+ FLAG_with_readline=
+ FLAG_without_readline=
+ true
+ case "$1" in
++ expr --prefix=/usr : '--prefix=\(.*\)'
+ FLAG_prefix=/usr
+ shift
+ true
+ case "$1" in
+ FLAG_with_readline=1
+ shift
+ true
+ case "$1" in
+ break
+ main
+ cc_quiet build/detect-cc.c
+ cc build/detect-cc.c -o /dev/null
+ mkdir -p _build
+ local out=_build/detected-config.sh
+ detect_and_echo_vars
+ test '' = 1
+ detect_readline
+ cc_quiet build/detect-readline.c -l readline
+ cc build/detect-readline.c -l readline -o /dev/null
+ test 1 = 1
+ die 'readline was not detected on the system (--with-readline passed).'
+ echo './configure ERROR: readline was not detected on the system (--with-readline passed).'
+ exit 1

This appears to be due to the detect-readline.c not including the relevant headers as documented in the manual (at least for readline 7.0):
I wish readline would get a .pc file...

diff --git a/build/detect-readline.c b/build/detect-readline.c
index 4b85e5d..b27ac29 100644
--- a/build/detect-readline.c
+++ b/build/detect-readline.c
@@ -1,4 +1,6 @@
+#include <stdio.h>
 #include <readline/readline.h>
+#include <readline/history.h>
 
 int main(void) {
   char *line = readline("");

After fixing this the initial ./configure command seems to succeed with the message:

./configure: Wrote _build/detected-config.sh

However when running make I am faced with the following issues:

% make
test -d _build/oil && \
  build/actions.sh app-deps oil ~/git/oil bin.oil
ln: failed to create symbolic link '/home/earnest/git/oil/_tmp': No such file or directory
test -d _build/hello && \
  build/actions.sh app-deps hello build/testdata hello
ln: failed to create symbolic link '/home/earnest/git/oil/_tmp': No such file or directory
make: *** No rule to make target '_build/oil/app-deps-c.txt', needed by '_build/oil/all-deps-c.txt'.  Stop.

This is just a simple laundry list of issues which could have been discussed elsewhere but github seemed like the only place.

@andychu
Copy link
Contributor

andychu commented Jul 31, 2017

Hey there, thanks for trying it and the report! About the forum, there is the oil-dev@ list, which is not very active at the moment, link on the home page:

http://www.oilshell.org/

Until there are a lot of people using Oil, I think e-mail will work better than IRC.

Can you first try the release tarball? I think you are building out of the repo, which is a bit more complicated.

http://www.oilshell.org/blog/2017/07/23.html

If you want to build out of the repo, try this:

https://github.com/oilshell/oil/wiki/Contributing

I would appreciate feedback on that, because I'm not sure if anyone has done it lately. There is the mkdir -p _tmp step, which I should probably get rid of. I think the Makefile can do that, since it also creates _bin and so forth.

./configure --prefix=/usr --with-readline simply exits with a return status of 1.

Hm it doesn't print an error message? If so, that's a bug.

This appears to be due to the detect-readline.c not including the relevant headers as documented in the manual (at least for readline 7.0):

I don't quite understand why adding headers would make it NOT fail? It's definitely possible that the detect-readline.c program has problems. But I am quite sure what happened in your case.

What platform are you building on?

@Earnestly
Copy link
Author

Earnestly commented Jul 31, 2017

What platform are you building on?

The platform is:

  • linux 4.12.4
  • glibc 2.25-819-g5920a4a624
  • readline 7.0.003
  • gcc 7.1.1

Can you first try the release tarball? I think you are building out of the repo, which is a bit more complicated.

I'd personally prefer to not need to use a release tarball, a good litmus test of any project is how well maintained and simple their build infrastructure is. If I have to use a release tarball for a build system issue, I'd consider that an issue.

If you want to build out of the repo, try this:

I think the release instructions should work as advertised. If this is still too early in the project's development then I can try again some other time.

Hm it doesn't print an error message?

It clearly tries to but the redirections in the configure script are incorrectly printing diagnostics to stdout instead of stderr. The log and die functions both redirect using 2>&1 instead of >&2 (the 1 is implied). Stdout is then later redirected in the call to detect_and_echo_vars so nothing is displayed and the error message ends up in the detected-config.sh file.

I don't quite understand why adding headers would make it NOT fail?

It mainly needs stdio.h:

% cc build/detect-readline.c -lreadline
In file included from /usr/include/readline/readline.h:35:0,
                 from build/detect-readline.c:1:
/usr/include/readline/rltypedefs.h:71:28: error: unknown type name ‘FILE’
 typedef int rl_getc_func_t PARAMS((FILE *));
                            ^
/usr/include/readline/readline.h:429:20: error: unknown type name ‘FILE’
 extern int rl_getc PARAMS((FILE *));
                    ^
In file included from build/detect-readline.c:1:0:
/usr/include/readline/readline.h:558:8: error: unknown type name ‘FILE’
 extern FILE *rl_instream;
        ^~~~
/usr/include/readline/readline.h:559:8: error: unknown type name ‘FILE’
 extern FILE *rl_outstream;
        ^~~~
/usr/include/readline/readline.h:588:8: error: unknown type name ‘rl_getc_func_t’
 extern rl_getc_func_t *rl_getc_function;
        ^~~~~~~~~~~~~~
/usr/include/readline/readline.h:917:3: error: unknown type name ‘FILE’
   FILE *inf;
   ^~~~
/usr/include/readline/readline.h:918:3: error: unknown type name ‘FILE’
   FILE *outf;
   ^~~~

Shame about IRC

andychu pushed a commit that referenced this issue Jul 31, 2017
@andychu
Copy link
Contributor

andychu commented Jul 31, 2017

OK, thanks for the report about readline:

8c1a6f1

There are two different sets of instructions:

  1. For the release tarball, use INSTALL.
  2. For the dev build, use the Contributing wiki page.

This is a well-known distinction. If you use autoconf, then configure will be in the tarball but not in the repo. configure.ac is in the repo, but not in the tarball. So you have to follow the right set of directions in any other project too.

The release tarball directions have been fairly well tested, although there are bugs in issue #16.

The dev instructions were changed recently and might need a little work. You actually don't need to run configure manually in this case, although it doesn't hurt. The makefile will run it for you.

@andychu
Copy link
Contributor

andychu commented Jul 31, 2017

OK I changed the Makefile so the _tmp hiccup should go away.

373f06b

@Earnestly
Copy link
Author

If you were going to use autotools then I'd have simply used autoreconf -i prior to running ./configure. This works for at least over a hundred projects I have personally packaged. I explicitly use their latest development releases (see glibc) to encounter potential bugs, especially integration issues.

@Earnestly
Copy link
Author

Earnestly commented Jul 31, 2017

Perhaps 373f06b fixed an issue with _tmp but it hasn't resolved the issue with make assuming ~/git. I know you want me to use the dev build instructions but currently due to their apparent and fairly stark divergence I'll simply decline for now.

Apologies for not complying, but I'll perhaps try again in a few months or when there's no longer two different build systems.

I'll close this for now as you have fixed the real main issue which was readline detection. Thanks.

Edit: I've also updated my first reply to explain more clearly why no error messages are being printed in the configure script.

@andychu
Copy link
Contributor

andychu commented Jul 31, 2017

Sure, but you said the instructions don't work, but I'm saying that every project has two sets of instructions, for a tarball vs. repo build. One of them might be implicit (do the standard autoconf thing), but it's still two different things.

FWIW the script to build the release tarball is here:

https://github.com/oilshell/oil/blob/master/scripts/release.sh

Although this will evolve. It's an explicit two-step build:

  1. repo -> tarball on dev machine (architecture-independent)
  2. tarball -> binary on user machine (architecture-dependent)

The developer build system will be complex for the forseeable future, see:

http://www.oilshell.org/blog/2017/05/05.html

http://www.oilshell.org/blog/2017/04/25.html

Unless someone wants to write an Python/Oil VM for me that is easy to build :)

However, the user build system is simple. Almost all of the complexity is hidden behind the release tarball. If I didn't have this two-step build (which almost all other projects have), then you would need to install Python to build Oil. I took care to make sure that Python wasn't a build-time dependency for the tarball.

@andychu
Copy link
Contributor

andychu commented Jul 31, 2017

OK, the ~/git thing is a bug. Let me think about how to fix that.

@andychu
Copy link
Contributor

andychu commented Jul 31, 2017

Also I don't see an edit in the original message about why configure is silently failing?

If --with-readilne fails, it should give an explicit error message.

@Earnestly
Copy link
Author

Earnestly commented Jul 31, 2017

_ Also I don't see an edit in the original message about why configure is silently failing?

It clearly tries to but the redirections in the configure script are incorrectly printing diagnostics to stdout instead of stderr. The log and die functions both redirect using 2>&1 instead of >&2 (the 1 is implied). Stdout is then later redirected in the call to detect_and_echo_vars so nothing is displayed and the error message ends up in the detected-config.sh file.

I don't agree that the build system needs to be complex at all, but it's your project.

Edit: Just to be clear, I was only trying to see how the build was coming since I had pretty high hopes for this effort. One of the main issues with sh in my opinion and usage is the lack of any decent way to synthesis command-line options without relying on wordsplitting behaviour (avoiding annoying IFS juggling). Oil represented a good idea as the author clearly knows about the virtues of make, awk, pipelines, command chaining and more -- something I find most people are entirely unaware of while being fairly derogatory to all of them.

I'm not much of a contributor, I had only noticed a few issues and wanted to quickly communicate my experience and IRC would have been a perfect channel for such inconsequential reporting. Either way, don't worry about me, focus your efforts on your proper contributors.

andychu pushed a commit that referenced this issue Jul 31, 2017
andychu pushed a commit that referenced this issue Jul 31, 2017
@andychu
Copy link
Contributor

andychu commented Jul 31, 2017

Ah that configure bug is embarrassing. Thanks for the report! I think that was a record number of bugs reported and fixed at once :)

The developer build system is complicated because I need Python to help me contain the complexity of bash. In other words, I shipped the Python prototype. There's no way I could have finished it in a reasonable amount of time in C or C++!

But this complexity is only for Oil developers -- I hid all the complexity for the system packagers and end users.

But I'm glad to get these little issues fixed so that people aren't put off from contributing.

However, developers have a 3rd way of running the code: just use the system Python interpreter, rather than the one I bundled with the release to avoid a Python dependency. That is what's documented in the Contributing wiki page. I actually hadn't documented how to make the tarball, although I gave a link to scripts/release.sh above.

Thanks for trying it!

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

2 participants