Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005737OCamlOCaml internal build/install (Makefiles, configure)public2012-08-24 00:262013-03-08 23:02
Reporterwilliam 
Assigned Tomeyer 
PrioritynormalSeverityfeatureReproducibilityalways
StatusassignedResolutionopen 
PlatformmxeOSlinuxOS Version
Product Version3.12.1 
Target VersionFixed in Version 
Summary0005737: adapt build system for cross-compilation
DescriptionHello,
I am using mxe on linux to cross-compile my application for windows. It uses camlimages, lablgtk2, lablgl, cairo, xml-light (and ocamlbuild+findlib)
Mxe is a platform for cross-compilation, it includes patches and make commands for hundreds of packages.
http://mxe.cc/ [^] (main site)
https://github.com/mxe/mxe [^] (sources)
https://github.com/william3/mxe [^] (fork being integrated)

To build ocaml I adapted debian patches maintained by Romain Beauxis, to make it usable with many different targets (i686-pc-mingw32, i686-w64-mingw32, x86_64-w64-mingw32 for 32/64bits targets).

Though it works fine with ocaml 3.12.1, I would like :
- to get your feed back if possible, to know if the process could be simplified (I do not really understand the patch)
- to know if you could integrate part of the code for next versions

Best regards,
William
TagsNo tags attached.
Attached Filespatch file icon 0001-various-patches-to-build-an-ocaml-cross-compiler.patch [^] (21,616 bytes) 2012-08-24 00:26 [Show Content]
patch file icon 0001-config-Makefile.mingw64-remove-redundant-mms-bitfiel.patch [^] (1,386 bytes) 2012-12-26 22:23 [Show Content]
patch file icon 0002-.gitignore-ignore-vim-temp-files-and-.cm-stuff.patch [^] (486 bytes) 2012-12-26 22:23 [Show Content]
patch file icon 0003-configure-use-stderr-for-all-messages-targetted-at-t.patch [^] (35,508 bytes) 2012-12-26 22:23 [Show Content]
patch file icon 0004-build-fix-filename-case.patch [^] (647 bytes) 2012-12-26 22:23 [Show Content]
patch file icon 0005-build-prefer-forward-slashes-to-backward-slashes.patch [^] (1,487 bytes) 2012-12-26 22:23 [Show Content]
patch file icon 0006-configure-fix-detection-of-non-working-C-compiler.patch [^] (1,581 bytes) 2012-12-26 22:24 [Show Content]
patch file icon 0007-configure-remove-unused-variable-from-case.in-test.patch [^] (2,419 bytes) 2012-12-26 22:24 [Show Content]
patch file icon 0008-configure-remove-unused-variable-from-case.in-test.patch [^] (1,364 bytes) 2012-12-26 22:24 [Show Content]
patch file icon 0009-configure-add-target-and-use-target-instead-of-host-.patch [^] (6,539 bytes) 2012-12-26 22:24 [Show Content]
patch file icon 0010-configure-when-cross-compiling-set-TOOLPREF-and-use-.patch [^] (6,731 bytes) 2012-12-26 22:24 [Show Content]
patch file icon 0011-configure-support-cross-compilation-with-mingw.patch [^] (9,166 bytes) 2012-12-26 22:24 [Show Content]
patch file icon 0012-windows-makefiles-remove-unused-DO-variable.patch [^] (1,347 bytes) 2012-12-26 22:24 [Show Content]
patch file icon 0013-windows-makefiles-use-dll-for-the-SO-extension.patch [^] (1,033 bytes) 2012-12-26 22:25 [Show Content]
gz file icon cross-patches-rev2-2012-12-29.tar.gz [^] (22,118 bytes) 2012-12-29 13:27
gz file icon patches_dump.tar.gz [^] (49,294 bytes) 2013-01-23 22:25
gz file icon cross-patches-rev2-2013-02-17.tar.gz [^] (54,029 bytes) 2013-02-17 22:48
gz file icon cross-patches-rev4-2013-02-24.tar.gz [^] (46,908 bytes) 2013-02-24 12:49

- Relationships

-  Notes
(0008020)
gasche (developer)
2012-09-04 16:18
edited on: 2012-09-04 16:18

I have no authority on the question of whether such patches could be accepted in a future state. I just had a look at the patches related to the OCaml compiler proper (very quick, so feel free to correct misunderstandings): I must they they're not really that convincing.

- some part of them amount to hardcode that the target is windows (see for example https://github.com/william3/mxe/blob/master/src/ocaml-core-1-fixes.patch#L589 [^] and below)

- some part of them are terribly hackish ("Give up and just define _finddata_t explicitly here")

- some part of them seems to be bugfixes or code changes that are independent of the cross-compilation question (I suppose you integrated all debian patches, not only those that straighten the build system). That seems to be the case of the ocamlbuild changes for example https://github.com/william3/mxe/blob/master/src/ocaml-ocamlbuild-1-fixes.patch [^]

I would understand their integration in a source tree designed solely for unix-to-windows compilation, with the understanding that they will remain as a fork forever. Even then, I would be a bit wary of this integration of all those independent changes in a single patch, making potential errors/divergences more difficult to track (that seems unrelated to your work and inevitable under the current project organization, though; they should consider moving to a structure with multiple patches per project).
But integrating them upstream is clearly impossible in their current state.

If I were a maintainer I wouldn't touch this until the changes are sorted out and given more structure and justification.

I also noticed that a lot of the "ocaml"-related patches do not concern the ocaml compiler proper: cairo, findlib, camlimages, xml-light, etc. are third-party libraries. If you have not yet contacted the upstream of those projects, you should consider doing so. A post on the caml-list may also interest people that would learn about how to make their programs portable with respect to this specific situation and, one can always hope, motivate a few charitable souls to help you getting the patches in better shape?

(0008023)
william (reporter)
2012-09-04 22:46

Hello,
Thanks for this analysis. My questions were only targeting this:
https://github.com/william3/mxe/blob/master/src/ocaml-core-1-fixes.patch [^]

Indeed, I tried to submit other patches to related people, but did not get many answers...

I would appreciate any hint that would improve the ocaml related patch :
https://github.com/william3/mxe/blob/master/src/ocaml-core-1-fixes.patch [^]
which is the patch given above :
http://caml.inria.fr/mantis/file_download.php?file_id=731&type=bug [^]

Changes to ocamlbuild were made mainly in order to make ocamlbuild usable for cross targets with ocamlfind integration.

Regards,
William
(0008649)
Camarade_Tux (reporter)
2012-12-26 22:32
edited on: 2012-12-26 22:32

Hi,

Based on William's patch, I made a series of smaller patches.

First one is: 0001-config-Makefile.mingw64-remove-redundant-mms-bitfiel.patch (and last one is 13th). The patch 0002 is something I forgot (I should have moved it out and not submit it). The patchs have some details if you select "show content" so I won't repeat it here. Some patches are trivial but for others, I have some questions on the approach.

* 0003-configure-use-stderr-for-all-messages-targetted-at-t.patch
Some things were printed to stdout and some others to stderr; I now send everything to stderr.

* 0010, 0011, 0013:
Are they a "right" approach? These changes need some polishing but is it ok to add special cases for cross-compilation with hardcoded settings?

Thanks,
Adrien Nader

(0008660)
meyer (developer)
2012-12-28 22:48
edited on: 2012-12-28 22:50

Hi,

Thanks for the patches. It looks like it's only a build system.

I reviewed them all, my comments below, however haven't attempted to cross compile OCaml yet. I might try it for ARM this weekend.
I think we need at least a second pair of eyes to look at the patches because they look really promising!

Comments
========

* patch 1

looks good apart from the part it would be good to:

- document why we are disabling the -mms-bitfields and what kind of
implications it has

- maybe making it configurable, like a variable for cross compilation
that is empty

* patch 2; ignored, spurious .gitignore/vim stuff

* patch 3

I admire simplicity and portability here, but we should use automatic redirection or custom command. Then if somebody adds another echo, she would be forced to use 'msg' command instead of echo and redirection I can see however that we use it in all the places, so looks Ok.

* patch 4
Looks OK.

* patch 5
Why do we need to touch Makefile.nt?

but looks OK. With a reservation it will be tested on Windows.

* patch 6
Looks OK.

* patch 7

I trust this cases remained the same, so looks ok.

* patch 8

Same as above.

* patch 9
Looks a bit hairy.
First of all why just not substitute host for target before it's been used?

It's certainly improvement to use it but increases a risk, of breaking some exotic builds.

* patch 10
We should initialise TOOLPREF in the begining if we plan to use it.

We should be able to replace these patterns:
*-gcc*
with something like:
*gcc*
thus avoiding a lot of redundant pattern matches

it might cause troubles only when something like 'mygcc' was used which is fairly unrelastic.

I think better way of using TOOLPREF would be to, once it's defined, redefine the interesting tools (as & gcc) to ${AS} & ${GCC} with TOOLPREF prefix and then use these variables.

* patch 11

This:

+ x86_64-*-mingw*) echo "Wow! A 64 bit architecture!" 1>&2
+ echo "#undef ARCH_SIXTYFOUR" >> m.h

doesn't sound like a win situation.

It will be more consistent to use variables like ${TOOLCHAIN}. Instead of ${toolchain} but that's just cosmetic.

I've gone through all the mingw cases, but can't guarantee they are correct until I would see myself ocaml crosscompiling.

* Patches 12,13
Looks OK.

(0008665)
Camarade_Tux (reporter)
2012-12-29 13:26
edited on: 2012-12-29 13:42

Thanks for the review.


So far I'm not cross-compiling OCaml successfully. By posting them here I'm trying to get the overall approach validated. There are also some changes that can be merged independently of the others and doing so will reduce the size of subsequent patches (including out-of-tree ones in the case cross-compilation support cannot be included easily).


I've updated the set of patches and I'm going to post them inside a tarball now or this page will soon be unreadable. I've reorganized the patches to move the least intrusive and least dangerous ones first so the patch numbers have changed but in this comment I'll use the same numbers as you did.


The file is named "cross-patches-rev2-2012-12-29.tar.gz".


I've dropped the patch about -mms-bitfields. This switch is now enabled by default for *-w64-mingw*. However this changed only happened in gcc 4.7.


I've introduced inf()/wrn()/err() functions. inf() writes its arguments to stderr; wrn() also writes "WARNING:\n" before; err() also "exit 2".


Forward slashes work just as well on windows. I've changed Makefile.nt to use forward slashes in advance. I'm not sure yet whether I'll be able to use the regular Makefiles or if I'll have to rely on Makefile.nt ones. I've moved the patch down in the list; we'll see a bit later.


Regarding "host" vs. "target" in the configure file, I simply started and went over all the cases using "$host". It turned out that all of them actually want the "target". However it's quite likely "host" will also be needed (probably for ocamlobjinfo for instance: it runs on $host and needs to link to a C library).


> I think better way of using TOOLPREF would be to, once it's defined, redefine the interesting tools (as & gcc) to ${AS} & ${GCC} with TOOLPREF prefix and then use these variables.

I've tried to do that but it touches areas for which I'm not sure I understand the intent. The biggest use of ${TOOLPREF}gcc is around line 830 and starts with:
  case "$arch,$system" in
    amd64,macosx) as="${TOOLPREF}as -arch x86_64"
                    aspp="${TOOLPREF}gcc -arch x86_64 -c";;
As far as I understand, ocaml requires aspp to come from gcc in this setup. Was that the intent or is it because the only compiler available was gcc? And what about clang/llvm?

I need some more input on that.


I've fixed the "#undef ARCH_SIXTYFOUR" issue and I've replaced "toolchain" with "TOOLCHAIN".


edit: I've fixed two issues but I'm not uploading another archive right now:
first,
# if target_type is "unknown", the script will try to cross-compile for the "unknown" architecture
-target_type=unknown
+target_type=""

second,
inf/wrn/err functions need to invoke echo with "-e" and need to quote the arguments

(0008671)
meyer (developer)
2012-12-30 04:04

Thanks for the patches, here are my further comments:

* patch 1

I like the cleanup you have made for DBM. I've applied this patch.

* patch 2

Looks good. Applied, thanks.

- patch 3

Not applied.

What kind of problems compatibilty.h gives?

If there are problems we should fix it in the header.

- patch 4

I like the attempt you've made to improve the ./configure script.

I'm also quite reserved about redirecting everything to stderr. Your
patch probably just changes how the user will interact with the script
as instead the user will filter the messages by string which might be
better or not.

By any means, I am tempted to apply these changes, once I know if there
are mandatory, thanks.

- Patch 5

I am resistant to warn the user, would be better to support both
notations? If you decide to do this, please do it for all the flags.

- patch 6

Is incorrect.

# cd config/auto-aux/
# cc=gcc sh ./runtest sizes.c
4 4 4 2

This will print to stdout the sizes of the primitive types.

Quotation will make them spliced into set builtin shell command, thus
setting the arguments to each of the sizes.

However you have issued:
# ret=$?
# set $ret

which just sets the first argument to the return code of the command.

* Patch 7

I'm resistant, looking closely I think it would be a premature
optimisation.

It's a small improvement that buys nothing, and might be reverted in a
future. Is there any particular reason for this cleanup?

* Patch 8

target_type is initially set to "unknown" therefore we shouldn't check
it with test -n. Also, we should print un-coditionaly information about
target, consistency here is more important. I also think we shouldn't
check-in mechanical renaming changes from $target to $host before we
have working cross-compilation so I'd refrain from checking-in the patch
now

* patch 9

Follow up of 4, so skipped for time being.

* patch 10

I wouldn't drop "support" for old buggy gcc versions, however in general
I am in favour of some cleanups.

Follow up of 4 so not applied yet.

* patch 11

Looks all right, applied.

* patches 12, 13

Don't have mingw system to test it, so I would rather not apply it
before I know it's important to change.


Overall, thank you for your efforts,

Wojciech
(0008672)
Camarade_Tux (reporter)
2012-12-30 16:53

- patch 3

The renaming from stat_alloc() to caml_stat_alloc() comes from William's patch. I haven't had an issue myself and thinking more about it, I'm not sure what would have caused an issue for William. I saw it as an opportunity for a slight update. I'm going to remove it for now; if "namespaced" function names are preferred, it will probably be better to change all of them and not only stat_alloc(), and do it at once. But for now, removed.


- patch 4

I started redirecting messages to stderr because there were some blocks of code that I wanted to use as functions (and therefore have them write to stdout (or stderr or anything)) but which were already writing to stdout. Usage between stderr and stdout was inconsistant but stderr seemed to prevail so I settled for that.

I think that I don't have the original issue anymore and I don't mind where the messages are sent as long as it's consistent. Progress messages are probably better on stdout but if warnings and errors go to stderr, they will probably in the wrong place or out of context. So, everything to stdout?

I think I'll try to redirect inf/wrn/err to "3" or something like that and use the "exec" command to redirect 3 to stdout. That should free stdout and stderr for use in other places and display properly in the terminal. (I'll see how it goes)

It could be good to use colour too for wrn and err but doing it in a portable fashion might be difficult. For later.


- patch 5

I added the check for "-option=val" vs. "-option val" because I made the mistake myself. The check is easy to implement but I'm not sure the support for "=" is. I'll see if '=' can be added to $IFS. However, it seems standard not to use '=' for ocaml


- patch 6

Very good catch. I'm wondering if the issue can be solved with "set". If not, I'll probably introduce size_short/size_long/size_... variables.


- patch 7

The unused variable in the case/in/esac constructs did not cause me any issue so far. I'll drop it.


- patch 8

I caught the issue with target initially being "unknown" but only after making the archive. I've fixed it by making the default "" instead of "unknown".

I'll make it always print the target.


- patch 10

I haven't dropped support for gcc 2.96/2.95. I've simply not touched them. This means the warnings won't appear if you cross-compile with gcc 2.96/29.5, which I consider an unlikely scenario.



Thanks a lot.
(0008785)
Camarade_Tux (reporter)
2013-01-23 22:26

Hi,

I've attached a new file. This is close to a code dump but I that way I'm somehow creating a backup for the patches. It's not really worth looking at them right now since they need cleaning.
(0008812)
meyer (developer)
2013-01-29 02:05

Dear Adrien,

Sorry to keep you waiting, I'm going to look at your cross-compilation patches this week.
(0008853)
Camarade_Tux (reporter)
2013-02-17 22:57

I've add "cross-patches-rev2-2013-02-17.tar.gz" (still "rev2" by mistake). I still need to clean things a bit but...

ocamlopt.opt works!


There's one thing I'm not verry happy with: I had to do changes asmcomp/ because it uses nativeints for many integers and I build on a 64bit system for a 32bit one; I ended up with several bad integer litterals in the generated assembly. This will definitely need proofreading.


Also, at first I had tried to cross-compile things like ocamldoc while building the cross-compiler. That didn't make much sense (that would only make sense if I were cross-compiling the compiler). In the end I added the ability to disable building ocamldoc and a few others like it is doable for camlp4.
So I did something that will improve cross-compiling the compiler but that is not needed for this bug report. What should I do about these patches?

Try to have them in? Try to have them but after the other ones? Forget about them (for now)?
Depending on the answer I'll reorganize the order of my patches to match so wait for rev4 before reading these patches.


Thanks. :-)
(0008855)
meyer (developer)
2013-02-18 03:10
edited on: 2013-02-23 17:43

Thanks for all the patches. My initial comments below, I said I applied
- but just realised after reviewing 31 small patches that there are 40 of
them, so I'll postpone it for tomorrow. The general comment is that it's
getting into the right track. Some of the patches needs a really
thorough reviewing before we commit them. The ones I marked with a
star are the ones that I think needs some improvements. I'm updating
this in-place as it goes along with applying and testing them. Short
note: did you try your cross-compile patches on a different
architecture, i.e ARM? Also we might try porting them to OPAM.
The reason why I ask is that I do have an ARM device, and would like to
build packages for it, and probably many other people would like.

Patch 1/
I don't think we should keep flexdll.h it in the triangle quotes - it's
not a system header.
However I think it's a part of OCaml installation so we should apply this patch.


Patch 2/
I trust it works, however I am not able to test it.


Patch 3/
Looks fine, thanks. I am not sure if we should prefix it with
underscore, my initial thought was that we shouldn't, but it's a header
not used anywhere else so maybe not worth to change.


Patch 4/
Looks OK to me, applied.


Patch 5/*
The echo -e command prints on my machine also leading -e.

$ ./configure --prefix $PWD
-e Configuring for host i686-pc-linux-gnu ...
-e Configuring for target i686-pc-linux-gnu ...
-e Using compiler gcc.


also the message prefix breaks the line:

./configure -help
ERROR:
-e Unknown option "-help".


The script continues to run even with error:

-e Using compiler arm-linux-gnueabi-gcc.
./tst: 1: Syntax error: word unexpected (expecting ")")
WARNING:
-e Unable to compile the test program.
-e This failure is expected for cross-compilation: we will assume the C compiler is ANSI-compliant.

-e Checking the sizes of integers and pointers...



Patch 6/
It's always best to inform the user about his failure. Applied.


Patch 7/
Applied. Thanks.


Patch 8/
I don't think we need $model anymore, so thanks for cleaning this up. I've applied this patch too.


Patch 9/
This needs to be documented. Probably additional entries to the manual
needs to be done. Especially that we are able to cross compile now at all.


Patch 10/
We should use conventional lowercase variable names in the Caml build
system. TOOLPREF and TOOLCHAIN all are not obeying the convention.

I suspect TOOLPREF/TOOLCHAIN are a mingw specific variable, but can't
find it anywhere. If we decide to expose it, then it needs to be
documented - I propose in the configure script itself as a commentary. But I'd leave it as it is.

My worry here is that we are hardcoding the sizes, instead of detecting
them.

I'll apply this patch however, because can't think about anything better
at the moment.


Patch 11/
I like this patch, applied. Did you try to test it on a different shell than bash?


Patch 12/*
I don't like this: '# XXX' prefix for the comment.

I'd prefer nicer prefix, like TODO or FIXME. However it's not a show
stopper, patch looks complicated so need to make a sweep over this
again.


Patch 13/
I don't have mingw system to test it, and there is 32 how about 64? If
there is another flavour of the library we need to cover that. I
tentatively applied.


Patch 14/
Looks OK.


Patch 15/*
ocaml -version can print other characters than specified in the sed command.

It would be much safer if the you matched for example like this:
sed 's/.*version \(.*\)/\1/g'

Also:
if test x"$target" != x"$host"; then
we should be using ==.


Patch 16/
Makes sense, applied.
It looks like a continuation of the previous patch, it would be easier to collate them.


Patch 17/
Looks OK, applied.


Patch 18/
Looks good to me, applied.


Patch 19/
Looks OK, applied.


Patch 20/
Fine. Applied.


Patch 21/
Apart from visible whitespaces it, looks OK.


EDIT: Sorry, I notice that the file have a lot leading whitespaces, I'll
clean them up as they polluting (not necessarily) the file.


Patch 22/
Thank you, applied.


Patch 23/
Don't put comma after `if' condition. Otherwise applied.


Patch 24/*
To be reviewed.


Patch 25/*
Yet to be reviewed again.


Patch 26/
Looks good to me. Thanks.


Patch 27/*
Needs to be reviewed.


Patch 28/*
I like it, but needs more looking.


Patch 29-40
Needs to be reviewed.

(0008856)
meyer (developer)
2013-02-18 04:32

> Try to have them in? Try to have them but after the other ones? Forget about them (for now)?
> Depending on the answer I'll reorganize the order of my patches to match so wait for rev4 before reading these patches.

We could consider them as an extra set of patches, if you separate them nicely, it would be a nice addition, but be careful to not over complicate what is now.
(0008862)
Camarade_Tux (reporter)
2013-02-18 23:18

Thanks for the comments.

I'll reorganize the commits so that the patches that could benefit cross-compilation of the compiler appear last.
I'll need a few more days to do that. Probably this week-end.

Btw, for TOOLPREF, it's not specific to anything. It's either arbitrary or already somewhere in the ocaml sources.
(0008863)
meyer (developer)
2013-02-18 23:52

Absolutely no rush, I'm going to look at it again in few days time - I have a fully booked week too.
(0008866)
william (reporter)
2013-02-19 11:42

about Camarade_Tux comment from 2013-02-17 22:57, regarding ocamldoc and camlp4 :

There are two categories of executables :

the ones that need to be cross compiled :
ocamlc ocamlrun ocamlcp ocamldep ocamlmklib ocamlmktop ocamlopt ocamlprof

the ones that do not need to be cross compiled :
camlp4prof camlp4boot camlp4 camlp4oof camlp4of camlp4o camlp4rf camlp4r camlp4orf ocamldoc ocamllex ocamlyacc

For all of them, it is important to be able to have them "prefixed", like
i686-w64-mingw32-ocamlc, even if this is for a program such as ocamldoc that is not really cross compiled. Because when building other libraries and programs, it is easier to use prefixed tools no matter if it is ocamldoc or ocamlc.

So would it be possible to have a build system that preserves building of those tools ?
(0008875)
Camarade_Tux (reporter)
2013-02-19 19:55

Well, it's difficult to have them build during the same build.

I believe it should be a requirement that if you want to build a cross ocaml X.Y.Z, you need to have a native ocaml X.Y.Z on your system.
The reason is fairly simple: there's no guarantee that it'll work otherwise. You might try with a different version of the compiler but you'll need a native ocaml compiler that is still fairly close version-wise. (plus you need the native compiler for camlp4 and ocamlbuild to work along with a working Dynlink)

That means that in order to build the cross-compiler, you will have to have a matching native toolchain. In that case you will be able to reuse most of it to get the right tools.

If you need to have specific names for these tools too, simply 'cp' or 'ln -s' them. They will exist for sure during build: they're really needed.

It might be doable to have them built as native tools during the build of the cross-compiler but it's really much more work. That will basically require that it is possible to cross-compile a native compiler. For now, 'cp' or 'ln -s' are very good ways to do it and actually the configure of the cross-compiler will fail if the version of the native compiler and the version of the cross-compiler you're building don't match.
(0008877)
meyer (developer)
2013-02-19 20:09

Dear William and Adrien,

Maybe a small chime in: building a cross-compiler does not preclude requirement of not having the compiler on the path. It does however simplify a lot the build process.

In general the problem could be solved by a separate recursive step for Makefile, so in normal world OCaml is built like this:

- bootstrap compiler builds bytecode compiler
- bytecode compiler builds bytecode compiler and the native one
- native compiler builds a native compiler compiled natively

in the cross-compilation world additional recursive invokation of Makefile would happen with all the variables set up to met the requirement building the cross-compiler.

In practice, it's not that simple however and I don't think it's needed at a cost of making it more complex. So I'd assume we already have the native toolchain already in place.
(0008881)
Camarade_Tux (reporter)
2013-02-20 07:47

I forgot to mention that both gfortran and gnat have the same requirement for cross-compilers and that's why I stopped trying to not require a native toolchain.
(0008887)
william (reporter)
2013-02-20 22:18

Indeed you are right.
In mxe, I ended up building first native ocaml then cross ocaml, copying native version of ocamldoc and camlp4* with right prefix.
Do not make things more complex, it is fine like that.
(0008891)
meyer (developer)
2013-02-23 17:27
edited on: 2013-02-23 17:29

I tried your patches with arm toolchain on x86:

$ ./configure --target arm-linux-gnueabi 
-e Configuring for host i686-pc-linux-gnu ...
-e Configuring for target arm-linux-gnueabi ...
-e Using compiler arm-linux-gnueabi-gcc.
./tst: 1: Syntax error: word unexpected (expecting ")")
WARNING:
-e Unable to compile the test program.
-e This failure is expected for cross-compilation: we will assume the C compiler is ANSI-compliant.

-e Checking the sizes of integers and pointers...
./tst: 1: Syntax error: word unexpected (expecting ")")
ERROR:
-e Unable to compile the test program.
 Make sure the C compiler 'arm-linux-gnueabi-gcc -O ' is properly installed.


2 things:
- tag prefix breaks lines - I included this comment in the summary
- the configure script continues running even though it failed - this was included in the previous-previous comments: exit will return from the shell function not from the script.

Also I would suggest testing on both on mingw and any other popular architecture (ARM for instance).

(0008896)
william (reporter)
2013-02-24 00:52

could you please explain how to patch sources with cross-patches-rev2-2013-02-17.tar.gz ?
I know the command :
patch -p1 < /path/to/patch
but how to use it for 30 patches ? I tried using "find /path/to/patch-dir -exec patch -p1 < {} \;", but does not work
Thanks
(0008897)
meyer (developer)
2013-02-24 01:41

William,
if you are sitting in the git repo, using git svn, then this command just works:

git am *.patch

if not than:

for f in *.patch; do patch -p1 < $f; done

(not that < {} will not work as it will splice two halves to redirection)
(0008900)
william (reporter)
2013-02-24 11:44

based on the git directory :
git clone http://github.com/ocaml/ocaml.git [^]
I do :
git am ../cross-patches-rev2-2013-02-17/*.patch

and get :
Applying: byterun/win32.c: use < > to #include flexdll.h instead of "".
Applying: windows: don't define lseeki64 and lseek since it already exists.
[...]
/home/performance/src/mxe/gits/ocaml/.git/rebase-apply/patch:16: space before tab in indent.
     debugger.o meta.o dynlink.o
warning: 1 line adds whitespace errors.
Applying: yacc: "ocamlyacc$(EXE)" rule produced "ocamlyacc" (no trailing $(EXE)).
Applying: .gitignore: ignore vim temp files and .cm* stuff.
Applying: STASH
error: patch failed: build/boot.sh:15
error: build/boot.sh: patch does not apply
Patch failed at 0037 STASH

same problem with git version of 17/02/2013
(0008901)
Camarade_Tux (reporter)
2013-02-24 12:49
edited on: 2013-02-24 12:50

Here's a new version of the patches. It's against HEAD~ (damn people commiting on sundays!) but with the patch in 0005887. The file is "cross-patches-rev4-2013-02-24.tar.gz".

I've moved commits around and I've squashed some of them together and got 31 commits (down from 40 or so).

There are three directories inside the archive:
* 01-non-specific/ : 10 commits
* 02-safe/ : 8 commits
* 03-wip/ : 13 commits

Their names should be fairly explicit. The 03-wip directory is included for completeness but is not mergeable.



If moved the error messages from "echo -e" to "printf '%b\n'" since echo isn't portable:
  http://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html [^]
  "It is not possible to use echo portably across all POSIX systems unless both -n (as the first argument) and escape sequences are omitted."

The fact that there is a newline after "WARNING:" and "ERROR!" is on purpose: I've found it to look nicer and be easier to stop. Actually, I've added a new line before the tag and another one after the error message.


I've improved some messages. In particular, the error you had with the arm toolchain is because the patches have a hardcoded list of integers/pointers/shorts/longs sizes when cross-compiling since they cannot be guessed. Currently, only windows 32bit and 64bit are there. I prefer to leave someone else add the entries for other toolchains.


I've documented -target in the INSTALL file.


The TOOLPREF/TOOLCHAIN variables are mostly arbitrary: they appear like that in some other projects but there is absolutely no rule about them. I haven't changed their name in these patches but there is no reason to not do it if it is wished.


I've tried the configure script with ash too (instead of bash invoked as sh) and had no issue.


About the use of the "ws2_32" library, the 32 is both for 32bit and 64bit. Windows 64 provides the "win32" API. It's a name, they could also have named it "windows-api-after-dos" and it's not going away nor changing in incompatible ways any time soon.


I've replaced the invocation of "ocamlrun -version" with "ocamlc -version":
  % ocamlrun -version
      The Objective Caml runtime, version 3.12.1
  % ocamlc -version
      3.12.1

That made it possible to replace sed with cut (in order to strip the +dev... part in the version).


For patch 15, you said:
  Also:
    if test x"$target" != x"$host"; then
  we should be using ==.
I don't understand: what is the issue?


For patch 23, you said:
  Don't put comma after `if' condition. Otherwise applied.

I've replaced:
  ifeq ($(var1), $(var2))
with:
  ifeq "$(var1)" "$(var2)"

Is it what you had in mind?

(0008902)
Camarade_Tux (reporter)
2013-02-24 13:00

Actually I've borked the change in byterun/Makefile.

I wrote:
  ifeq $(SYSTEM) mingw
but it should have been:
  ifeq "$(SYSTEM)" "mingw"

The corresponding file is 02-safe/0014-build-select-win32-variants-of-unix-and-graph-for-mi.patch .
(0008908)
meyer (developer)
2013-02-25 03:17

Adrien, thanks for the next round of patches.

My answers for your questions:

> For patch 23, you said:
> Don't put comma after `if' condition. Otherwise applied.

My bad typo, I actually meant: don't put space after the comma. The space after comma is significant. Correct syntax is:
  ifeq ($(SYSTEM),mingw)

I'd not use cut, sed seems to be better tool for that, but you are right if we want just base version than probably the regex you submitted is OK.
(0008909)
meyer (developer)
2013-02-25 04:05

Adrien,

I tested and committed first 5 patches from 01-non-specific/.

Please excuse doing that piece by piece, but I am also reviewing them during the tour. The next commits will contain the rest of the patches from 01-non-specific.
(0008933)
meyer (developer)
2013-02-28 00:30
edited on: 2013-02-28 00:42

Adrien,

A small update:

Sorry, as usually, I got very limited means of time, to commit and test the patches, but in essence I really like what you did with sub-dividing them. I will do my best to commit the rest from 01-non-specific/ during the week so we can move on with the rest of the patches. I will look if the compilation actually worked on anything else than mingw, like ARM.

(0008957)
meyer (developer)
2013-03-08 23:02
edited on: 2013-03-08 23:35

Patch 6/

inf() {
  printf "%b\n" "$*" 1>&3
}

wrn() {
  printf "\nWARNING:\n" 1>&3
  printf "%b\n\n" "$*" 1>&3
}

err() {
  printf "\nERROR!\n" 1>&3
  printf "%b\n\n" "$*" 1>&3
  exit 2
}

exec 3>&1


First you are writing to a custom descriptor 3, and then you will redirect everything from 3 to stdout (descriptor 1). What's the purpose?


Patch 7/
Modified patch looks good:

-if echo $configure_options | grep -q ' --\?[a-zA-Z0-9-]\+='; then
+if echo "$configure_options" | grep -qe '--\?[a-zA-Z0-9-]\+='; then

two things:

- added missing quotes
- and -e flag to escape leading --, and dropped the space

Patch 8/
Not applied.

Sporious lines here:

+  amd64,macosx)   as='as -arch x86_64'
+                  aspp='gcc -arch x86_64 -c';;



- Issue History
Date Modified Username Field Change
2012-08-24 00:26 william New Issue
2012-08-24 00:26 william File Added: 0001-various-patches-to-build-an-ocaml-cross-compiler.patch
2012-09-04 16:18 gasche Note Added: 0008020
2012-09-04 16:18 gasche Note Edited: 0008020 View Revisions
2012-09-04 16:20 gasche Status new => acknowledged
2012-09-04 22:46 william Note Added: 0008023
2012-12-26 22:23 Camarade_Tux File Added: 0001-config-Makefile.mingw64-remove-redundant-mms-bitfiel.patch
2012-12-26 22:23 Camarade_Tux File Added: 0002-.gitignore-ignore-vim-temp-files-and-.cm-stuff.patch
2012-12-26 22:23 Camarade_Tux File Added: 0003-configure-use-stderr-for-all-messages-targetted-at-t.patch
2012-12-26 22:23 Camarade_Tux File Added: 0004-build-fix-filename-case.patch
2012-12-26 22:23 Camarade_Tux File Added: 0005-build-prefer-forward-slashes-to-backward-slashes.patch
2012-12-26 22:24 Camarade_Tux File Added: 0006-configure-fix-detection-of-non-working-C-compiler.patch
2012-12-26 22:24 Camarade_Tux File Added: 0007-configure-remove-unused-variable-from-case.in-test.patch
2012-12-26 22:24 Camarade_Tux File Added: 0008-configure-remove-unused-variable-from-case.in-test.patch
2012-12-26 22:24 Camarade_Tux File Added: 0009-configure-add-target-and-use-target-instead-of-host-.patch
2012-12-26 22:24 Camarade_Tux File Added: 0010-configure-when-cross-compiling-set-TOOLPREF-and-use-.patch
2012-12-26 22:24 Camarade_Tux File Added: 0011-configure-support-cross-compilation-with-mingw.patch
2012-12-26 22:24 Camarade_Tux File Added: 0012-windows-makefiles-remove-unused-DO-variable.patch
2012-12-26 22:25 Camarade_Tux File Added: 0013-windows-makefiles-use-dll-for-the-SO-extension.patch
2012-12-26 22:32 Camarade_Tux Note Added: 0008649
2012-12-26 22:32 Camarade_Tux Note Edited: 0008649 View Revisions
2012-12-28 22:48 meyer Note Added: 0008660
2012-12-28 22:50 meyer Note Edited: 0008660 View Revisions
2012-12-29 13:26 Camarade_Tux Note Added: 0008665
2012-12-29 13:27 Camarade_Tux File Added: cross-patches-rev2-2012-12-29.tar.gz
2012-12-29 13:27 Camarade_Tux Note Edited: 0008665 View Revisions
2012-12-29 13:42 Camarade_Tux Note Edited: 0008665 View Revisions
2012-12-30 04:04 meyer Note Added: 0008671
2012-12-30 04:13 meyer Assigned To => meyer
2012-12-30 04:13 meyer Status acknowledged => assigned
2012-12-30 16:53 Camarade_Tux Note Added: 0008672
2013-01-23 22:25 Camarade_Tux File Added: patches_dump.tar.gz
2013-01-23 22:26 Camarade_Tux Note Added: 0008785
2013-01-29 02:05 meyer Note Added: 0008812
2013-02-17 22:48 Camarade_Tux File Added: cross-patches-rev2-2013-02-17.tar.gz
2013-02-17 22:57 Camarade_Tux Note Added: 0008853
2013-02-18 03:10 meyer Note Added: 0008855
2013-02-18 04:32 meyer Note Added: 0008856
2013-02-18 05:02 meyer Note Edited: 0008855 View Revisions
2013-02-18 23:18 Camarade_Tux Note Added: 0008862
2013-02-18 23:52 meyer Note Added: 0008863
2013-02-19 11:42 william Note Added: 0008866
2013-02-19 19:55 Camarade_Tux Note Added: 0008875
2013-02-19 20:09 meyer Note Added: 0008877
2013-02-20 07:47 Camarade_Tux Note Added: 0008881
2013-02-20 22:18 william Note Added: 0008887
2013-02-23 16:36 meyer Note Edited: 0008855 View Revisions
2013-02-23 16:38 meyer Note Edited: 0008855 View Revisions
2013-02-23 16:41 meyer Note Edited: 0008855 View Revisions
2013-02-23 17:03 meyer Note Edited: 0008855 View Revisions
2013-02-23 17:15 meyer Note Edited: 0008855 View Revisions
2013-02-23 17:15 meyer Note Edited: 0008855 View Revisions
2013-02-23 17:17 meyer Note Edited: 0008855 View Revisions
2013-02-23 17:27 meyer Note Added: 0008891
2013-02-23 17:29 meyer Note Edited: 0008891 View Revisions
2013-02-23 17:43 meyer Note Edited: 0008855 View Revisions
2013-02-24 00:52 william Note Added: 0008896
2013-02-24 01:41 meyer Note Added: 0008897
2013-02-24 11:44 william Note Added: 0008900
2013-02-24 12:49 Camarade_Tux Note Added: 0008901
2013-02-24 12:49 Camarade_Tux File Added: cross-patches-rev4-2013-02-24.tar.gz
2013-02-24 12:50 Camarade_Tux Note Edited: 0008901 View Revisions
2013-02-24 13:00 Camarade_Tux Note Added: 0008902
2013-02-25 03:17 meyer Note Added: 0008908
2013-02-25 04:05 meyer Note Added: 0008909
2013-02-28 00:30 meyer Note Added: 0008933
2013-02-28 00:40 meyer Note Edited: 0008933 View Revisions
2013-02-28 00:42 meyer Note Edited: 0008933 View Revisions
2013-03-08 23:02 meyer Note Added: 0008957
2013-03-08 23:03 meyer Note Edited: 0008957 View Revisions
2013-03-08 23:03 meyer Note Edited: 0008957 View Revisions
2013-03-08 23:11 meyer Note Edited: 0008957 View Revisions
2013-03-08 23:12 meyer Note Edited: 0008957 View Revisions
2013-03-08 23:33 meyer Note Edited: 0008957 View Revisions
2013-03-08 23:35 meyer Note Edited: 0008957 View Revisions


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker