Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006475OCamlOCaml generalpublic2014-06-30 11:302016-02-25 15:55
Reporteryallop 
Assigned Towhitequark 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusconfirmedResolutionreopened 
PlatformOSOS Version
Product Version 
Target Version4.03.1+devFixed in Version 
Summary0006475: -o is ignored for C files
DescriptionThe -o option is ignored when compiling C files, which makes it slightly awkward to write object files to a directory other than the current directory.

For example, the following command

   ocamlc -c -o build/code.cmo src/code.ml

creates 'build/code.cmo' as expected, but the corresponding command for a C file

   ocamlc -c -o build/code.o src/code.c

creates 'code.o' in the current directory.
Tagspatch
Attached Filespatch file icon ocamlc-c-o.patch [^] (4,616 bytes) 2014-12-21 13:02 [Show Content]

- Relationships
related to 0005890acknowledged ocamlc does not fully take into consideration -o parameter 

-  Notes
(0012423)
whitequark (developer)
2014-10-23 22:14

I should add that ocamlbuild should be fixed as well when this issue is resolved. It currently uses mv to move the .o from the current directory.
(0012429)
yallop (developer)
2014-10-24 15:27

There's a patch by @vbgl here:

   https://github.com/ocaml/ocaml/pull/103 [^]
(0012902)
gasche (developer)
2014-12-21 11:50

whitequark, could you write a test case in the ocamlbuild testsuite to make sure your ocamlbuild patch works as expected? There already is some very light output-obj testing in internal.ml.
(0012907)
whitequark (developer)
2014-12-21 13:02

I have attached a new patch that contains:

  * @vbgl's fix
  * changes to ocamlbuild
  * ocamlbuild test
  * changes to Changes
(0012908)
gasche (developer)
2014-12-21 13:20
edited on: 2014-12-21 13:21

I merged the (very nice!) patch, but in trunk only rather than in the stable branch. My reasoning is that other build systems may have workarounds for this issue (we had to fix ocamlbuild) which may break now that it is fixed.

(We don't know yet whether there will be another minor release, but Damien seems to be thinking about it.)

Thanks to all of you for the report and bugfixes.

(0012909)
whitequark (developer)
2014-12-21 13:22

I believe this can be merged in stable, because the patch does not change the behavior *without -o*, and other buildsystems are unlikely to pass an option that does nothing. So the workarounds will still work.
(0012920)
whitequark (developer)
2014-12-21 19:05

(In other words we did not *have* to fix ocamlbuild, I just thought it would be much cleaner.)
(0012921)
gasche (developer)
2014-12-21 19:11

Okay, will merge in 4.02 at a later date.
(0012993)
gasche (developer)
2014-12-27 10:23

Merged in 4.02.
(0013324)
doligez (administrator)
2015-02-21 00:22

There is a problem with this change: it breaks (at least) ocamlnet because the ocamlnet config script does this:
```
ocamlc -custom -o t tend.c t.ml
```
And with this patch, the C compiler puts its output in `t` instead of putting it in `tend.o` where the OCaml compiler expects it.

The long-term solution is for users to be aware that the order of arguments is important and to write instead:
```
ocamlc -custom tend.c t.ml -o t
```
I wouldn't mind doing this in trunk and documenting it properly ("sets the name of the output file for the next source file, or for the whole compilation if it appears on the command line later than all sources files") but for 4.02 I think it's too much incompatibility, so we should roll back.

We should also probably reset the -o argument as soon it has been used for one source file, since it doesn't make sense to overwrite a file you have just generated.

Thoughts?
(0013325)
whitequark (developer)
2015-02-21 00:28

No, I think that the current behavior is actually wrong. There is no reason whatsoever to set the output file name for one of the intermediate build products. Indeed, the -o option should always set the filename for the whole compilation.
(0013326)
gasche (developer)
2015-02-21 14:12

I just reverted (4.02@15863) the 4.02 commit. The behavior in trunk is still problematic, so I'll revert it as well if don't converge on a solution.
(0013328)
doligez (administrator)
2015-02-23 19:12

@whitequark, I thought we had decided it would be nice to be able to do:

  ocamlopt -c -o /tmp/foo.o foo.c -o /tmp/bar.cmx bar.ml

Or maybe that's overkill because no build system will do that. In that case, we still want -o to work if there's only one source file and the -c option is also given.
(0013330)
whitequark (developer)
2015-02-23 22:39

I think that's overkill and is very confusing. No other compiler in existence does that, and even few Unix tools do.

Indeed; with -c, that would qualify as the end result of compilation.
(0015265)
nevor (reporter)
2016-01-21 15:07
edited on: 2016-01-21 15:28

The commit seems to have been reverted after the merge into trunk. The current behavior (still) breaks library generation from mixed C/OCaml files as described by doligez.

(0015273)
frisch (developer)
2016-01-27 09:20

I'm raising the Severity, since this is an actual regression from the previous release. We should either revert the change or find a proper solution.
(0015300)
frisch (developer)
2016-02-02 10:28

I propose to revert this today, unless someone objects to it (or, better, fixes the issue).
(0015301)
whitequark (developer)
2016-02-02 11:38

I object to reverting. Is it hard to make a change so that the name for the whole compilation is set? I would do it, but I am trying to move internationally right now, so a bit busy here. I can do it in about three days.
(0015302)
frisch (developer)
2016-02-02 11:39

> Is it hard to make a change so that the name for the whole compilation is set?

I don't know, I did not look at it. I only observe that the current behavior can introduce regression in the wild (concretely, at LexiFi, we had to fix on Makefile locally after synchronizing with trunk) and that it should be fixed before the release. I'll let Damien decide on what to do.
(0015321)
whitequark (developer)
2016-02-06 15:32

Damien, any thought on this? I will be able to spend some time on this soon.
(0015325)
doligez (administrator)
2016-02-08 17:35

What the compiler should do is clear:
- if `-o` is given but not `-c` (set the file name for the final product only)
- if `-o` and `-c` are given with only one source file (set the file name for the sole output file)

What is less clear is what to do when `-o` and `-c` are given, and several source files are present. If you can make that an error (similar to `ocamlopt -shared` without `-o`) I think we're good.

More generally, you want to trigger an error instead of overwriting an output file that was just written by the same command.

4.03.0 will be released within one month, so you should propose a patch within the next few weeks. And make it as simple as possible, so we can be sure it doesn't break something else.
(0015331)
whitequark (developer)
2016-02-09 15:20

Please see https://github.com/ocaml/ocaml/pull/464. [^]

- Issue History
Date Modified Username Field Change
2014-06-30 11:30 yallop New Issue
2014-07-11 13:20 doligez Severity minor => feature
2014-07-11 13:20 doligez Status new => acknowledged
2014-10-23 22:14 whitequark Note Added: 0012423
2014-10-24 15:27 yallop Note Added: 0012429
2014-10-24 15:27 yallop Tag Attached: patch
2014-12-20 13:23 whitequark File Added: ocamlc-c-ocamlbuild.patch
2014-12-21 11:50 gasche Assigned To => whitequark
2014-12-21 11:50 gasche Status acknowledged => assigned
2014-12-21 11:50 gasche Note Added: 0012902
2014-12-21 13:02 whitequark File Deleted: ocamlc-c-ocamlbuild.patch
2014-12-21 13:02 whitequark File Added: ocamlc-c-o.patch
2014-12-21 13:02 whitequark Note Added: 0012907
2014-12-21 13:20 gasche Note Added: 0012908
2014-12-21 13:21 gasche Note Edited: 0012908 View Revisions
2014-12-21 13:21 gasche Note Edited: 0012908 View Revisions
2014-12-21 13:22 whitequark Note Added: 0012909
2014-12-21 19:05 whitequark Note Added: 0012920
2014-12-21 19:11 gasche Note Added: 0012921
2014-12-27 10:23 gasche Note Added: 0012993
2014-12-27 10:23 gasche Status assigned => resolved
2014-12-27 10:23 gasche Fixed in Version => 4.02.2+dev / +rc1
2014-12-27 10:23 gasche Resolution open => fixed
2015-02-21 00:22 doligez Note Added: 0013324
2015-02-21 00:22 doligez Status resolved => feedback
2015-02-21 00:22 doligez Resolution fixed => reopened
2015-02-21 00:22 doligez Target Version => 4.02.2+dev / +rc1
2015-02-21 00:28 whitequark Note Added: 0013325
2015-02-21 14:12 gasche Note Added: 0013326
2015-02-23 19:12 doligez Note Added: 0013328
2015-02-23 22:39 whitequark Note Added: 0013330
2015-04-02 18:00 doligez Fixed in Version 4.02.2+dev / +rc1 =>
2015-04-02 18:00 doligez Target Version 4.02.2+dev / +rc1 => 4.03.0+dev / +beta1
2016-01-21 15:07 nevor Note Added: 0015265
2016-01-21 15:12 nevor Note Edited: 0015265 View Revisions
2016-01-21 15:28 nevor Note Edited: 0015265 View Revisions
2016-01-27 09:20 frisch Note Added: 0015273
2016-01-27 09:20 frisch Severity feature => major
2016-01-27 09:20 frisch Status feedback => confirmed
2016-02-02 10:28 frisch Note Added: 0015300
2016-02-02 11:38 whitequark Note Added: 0015301
2016-02-02 11:39 frisch Note Added: 0015302
2016-02-06 15:32 whitequark Note Added: 0015321
2016-02-08 17:35 doligez Note Added: 0015325
2016-02-09 15:20 whitequark Note Added: 0015331
2016-02-10 15:54 doligez Relationship added related to 0005890
2016-02-25 15:55 doligez Target Version 4.03.0+dev / +beta1 => 4.03.1+dev


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker