Opened 5 years ago
Closed 5 years ago
#1655 closed defect (fixed)
csc: when .c files are supplied and -c is given, the -o option has no effect
Reported by: | kristianlm | Owned by: | felix winkelmann |
---|---|---|---|
Priority: | major | Milestone: | 5.2 |
Component: | core tools | Version: | 5.1.0 |
Keywords: | Cc: | ||
Estimated difficulty: | medium |
Description
as illustrated here,
$ csc -c hello.scm library.c -o hello.o.myextension
produces hello.o.o, not hello.o.myextension as expected
Change History (16)
comment:1 Changed 5 years ago by
Component: | unknown → compiler |
---|---|
Estimated difficulty: | → easy |
Milestone: | someday → 5.2 |
comment:2 Changed 5 years ago by
Component: | compiler → core tools |
---|---|
Owner: | set to felix winkelmann |
Status: | new → assigned |
comment:3 Changed 5 years ago by
What is the expected behaviour here? You are passing two files but want to produce one object file, so I assume you want both to be linked together? AFAIK it is not possible to combine multiple .o files in another .o file, unless you produce a library (static or dynamic). gcc
, for example, produces this, when given multiple files + the -c
option:
gcc: cannot specify -o with -c or -S with multiple files
Perhaps we should produce the same error.
comment:4 Changed 5 years ago by
Good point, and thanks for looking into this. I don't know what I want, other than tweetnacl to cross-compile properly. In there, you have:
klm@kth /tmp ➤ chicken-install -r tweetnacl tweetnacl located at /home/klm/.cache/chicken-install/tweetnacl klm@kth /tmp ➤ cd /home/klm/.cache/chicken-install/tweetnacl/ klm@kth ~/.c/c/tweetnacl ➤ cat build-tweetnacl #!/bin/sh -e "$CHICKEN_CSC" -O2 -d1 -C "$CFLAGS" -L "$LDFLAGS" tweetnacl.impl.c "$@"
When cross compiling this egg, the "$@"
contains a -o 'tweetnacl.o.target'
so that is where this situation is happening.
Is there now a way to specify C sources in the .egg
file? If so, that is probably the correct way to fix this. The only reason Thomas is using a custom build script is to compile the C source file in there.
Otherwise, perhaps a sensible solution is to say that each -o
applies to the first source-file preceding it or something like that?
comment:5 Changed 5 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed by 90b57243, which makes this an error condition with a clear message.
comment:6 Changed 5 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
This breaks in several places where custom-build scripts are used, and probably more. The general problem is that the static compilation uses -c
and chicken-install passes the additional source dependencies as arguments, which is incorrect.
comment:7 Changed 5 years ago by
Estimated difficulty: | easy → medium |
---|
comment:8 Changed 5 years ago by
To reproduce the error this change triggers, consider the tweetnacl egg: The problem here is that the custom-build script adds an additional file to the csc invocation, taking advantage of the fact that csc will compile all files given on the command line. Previously the "-o" option was partially ignored when multiple files together with the "-c" where given, which seemed to work most of the time, or accidentally met the user's expectations. But strictly speaking, the custom build script is incorrect.
comment:9 Changed 5 years ago by
(ignore my remark about additional dependencies being added - that was a wrong assumption)
comment:10 Changed 5 years ago by
I think that the patch to error out is right here, and that this method of adding a C file to the custom-build script is wrong.
It only happens to work when building a shared object, but doesn’t work when building a static object (csc is not making an archive file) or when cross-compiling, as Kristian originally reported.
The two proper methods of linking a C library to a Scheme library are: #include <the_c_file>
inside a foreign-declare
for example, or using the c-object
component to build the C library and objects
component property to link that C library to the Scheme one.
The first method always works, the second requires CHICKEN 5.1.0 or later.
comment:11 Changed 5 years ago by
Fixed with 90b57243dbf25a2b0e32114746d593a8599e1528; but some eggs still need to be fixed
comment:12 Changed 5 years ago by
Here is a list of eggs that are broken by this change:
- endian-blob (patch sent)
- random-mtzig (patch sent)
- rbf (patch sent)
- sql-de-lite
statistics(fixed in svn)- webview
- genann (patch sent)
- spiffy-cgi-handlers (patch sent)
sqlite3pth(fixed)
comment:13 Changed 5 years ago by
Kooda sent a patch for spiffy-cgi-handlers which I have applied and tagged as 0.7.
comment:14 Changed 5 years ago by
Felix sent me a diabolically clever kludge for sql-de-lite which maintains compatibility with 5.0.0, but I'm wondering if we should just fix it the "right" way (c-object) even though it means a minimum of 5.1.0? Any thoughts?
This assumes the c-object fix works here; I haven't looked at it yet. This build script is terribly complicated and got even more so when modified for Chicken 5.
comment:15 Changed 5 years ago by
I think it is for the egg-author to decide whether breaking compatibility with 5.0.0 is acceptable. I'll provide a c-objects patch, if I can and then we can see what's best.
comment:16 Changed 5 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Not completely fixed in all eggs yet, but it's not a blocker for the release.
Should be relatively easy to fix, nice to get this into 5.2