Opened 4 years ago

Closed 4 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 4 years ago by sjamaan

Component: unknowncompiler
Estimated difficulty: easy
Milestone: someday5.2

Should be relatively easy to fix, nice to get this into 5.2

comment:2 Changed 4 years ago by felix winkelmann

Component: compilercore tools
Owner: set to felix winkelmann
Status: newassigned

comment:3 Changed 4 years ago by felix winkelmann

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 4 years ago by kristianlm

Good point. 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?

Version 0, edited 4 years ago by kristianlm (next)

comment:5 Changed 4 years ago by evhan

Resolution: fixed
Status: assignedclosed

Fixed by 90b57243, which makes this an error condition with a clear message.

comment:6 Changed 4 years ago by felix winkelmann

Resolution: fixed
Status: closedreopened

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 4 years ago by felix winkelmann

Estimated difficulty: easymedium

comment:8 Changed 4 years ago by felix winkelmann

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 4 years ago by felix winkelmann

(ignore my remark about additional dependencies being added - that was a wrong assumption)

comment:10 Changed 4 years ago by Kooda

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 4 years ago by sjamaan

Fixed with 90b57243dbf25a2b0e32114746d593a8599e1528; but some eggs still need to be fixed

comment:12 Changed 4 years ago by Kooda

Here is a list of eggs that are broken by this change:

  • endian-blob (fixed)
  • random-mtzig (fixed)
  • rbf (fixed)
  • sql-de-lite
  • statistics (fixed in svn)
  • webview
  • genann (patch sent)
  • spiffy-cgi-handlers (fixed)
  • sqlite3pth (fixed)
Last edited 4 years ago by Kooda (previous) (diff)

comment:13 Changed 4 years ago by andyjpb

Kooda sent a patch for spiffy-cgi-handlers which I have applied and tagged as 0.7.

comment:14 Changed 4 years ago by Jim Ursetto

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 4 years ago by felix winkelmann

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 4 years ago by sjamaan

Resolution: fixed
Status: reopenedclosed

Not completely fixed in all eggs yet, but it's not a blocker for the release.

Note: See TracTickets for help on using tickets.