SquircleSpace / shcl

SHell in Common Lisp
Apache License 2.0
318 stars 17 forks source link

Miscellaneous build fixes #27

Closed ahefner closed 5 years ago

SquircleSpace commented 5 years ago

I think GCC is pickier by default than clang. I'm seeing this error in the CI builds:

core/support/spawn.c: In function ‘shcl_fd_actions_take’:
core/support/spawn.c:105:5: error: ‘for’ loop initial declarations are only allowed in C99 mode
     for (shcl_fd_action_node *node = actions->head; NULL != node; node = node->next) {
     ^
core/support/spawn.c:105:5: note: use option -std=c99 or -std=gnu99 to compile your code
core/support/spawn.c: In function ‘shcl_spawn’:
core/support/spawn.c:151:5: error: ‘for’ loop initial declarations are only allowed in C99 mode
     for (int i = 0; NULL != envp[i]; ++i) {
     ^

Fair point, GCC! We should be declaring an explicit standard that we're conforming to. Let's toss a --std=C11 into the arg list.

SquircleSpace commented 5 years ago

Hm. I've suspected that this defconstant error was a compiler bug. I've been meaning to try to dissect the failure and send SBCL a useful bug report, but now I think I might have been wrong.

The SBCL manual recommends using symbol-value as you do here. They don't explain why they use symbol-value, but I'm guessing the author put it there for a reason! I dug up the commit where they introduced that recommendation, but it doesn't provide any extra insight. https://github.com/sbcl/sbcl/commit/3c0ee1b87c4191298a8842cf7682c3f308680e66

After reading some of the ANSI standard and a few parts of SBCL, I'm unsure whether the code I wrote is technically legal. Rather than fight the SBCL interpretation of the standard, I'll take SBCL's recommendation for how to deal with this.

SquircleSpace commented 5 years ago

I'm trying to ensure that CI builds always pass for the master branch, and the -std=C11 issue prevents builds from passing. So, I'm going to merge this into the unstable branch and make a few tweaks before promoting it to master.

ahefner commented 5 years ago

Whoops. I was building it with -std=c99, but I forgot to put that in the PR.

On Fri, Jul 19, 2019 at 7:30 PM Brad Jensen notifications@github.com wrote:

Merged #27 https://github.com/bradleyjensen/shcl/pull/27 into unstable.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bradleyjensen/shcl/pull/27?email_source=notifications&email_token=AABLQMOE2XWQGGVJ3HBONKLQAJFBHA5CNFSM4IFCEBB2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOSTMJKBA#event-2497221892, or mute the thread https://github.com/notifications/unsubscribe-auth/AABLQMN6JLVO2M6GSUBH66LQAJFBHANCNFSM4IFCEBBQ .