Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Coin3D #966

Closed
wants to merge 4 commits into from
Closed

Add Coin3D #966

wants to merge 4 commits into from

Conversation

fiesh
Copy link

@fiesh fiesh commented Nov 3, 2015

This adds the Coin3D framework, a BSD licensed implementation of Open Inventor (http://www.openinventor.com/) to MXE..

@@ -33,6 +33,7 @@
"cimg": "1.6.3",
"cmake": "3.0.2",
"cminpack": "1.3.4",
"coin": "3.1.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is updated by @MXEBot

@starius
Copy link
Member

starius commented Nov 3, 2015

Please squash 8889b3a

@fiesh
Copy link
Author

fiesh commented Nov 3, 2015

Done

@starius
Copy link
Member

starius commented Nov 3, 2015

I think name coin3d is better than coin.

define $(PKG)_BUILD
cd '$(1)' && ./configure \
$(MXE_CONFIGURE_OPTS) \
--disable-debug \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces and tabs mixture.

@starius
Copy link
Member

starius commented Nov 3, 2015

The package installs files usr/<target>/bin/coin-config which are equal for all targets. Are these files needed? I think, this file should not be installed or be installed to usr/bin/coin-config.

@starius
Copy link
Member

starius commented Nov 3, 2015

Files created by package coin: https://gist.github.com/starius/9169f54b4f89ea74c3a7

--without-zlib \
--without-bzip2 \
--without-x
$(MAKE) -C '$(1)' -j 1 uninstall
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you make uninstall?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this from x264.mk, supposing there was a reason for this. I guess it can be omitted.

@starius
Copy link
Member

starius commented Nov 3, 2015

Please add a test executable using this library and add a command to build it. It is needed to check linking errors.

@fiesh
Copy link
Author

fiesh commented Nov 3, 2015

I don't agree with the name coin3d, gentoo or opensuse call it "coin", so I think sticking to names in use is best.

@fiesh
Copy link
Author

fiesh commented Nov 3, 2015

I'll set up a test, but I will be busy tomorrow, so please be patient.

@starius
Copy link
Member

starius commented Nov 3, 2015

gentoo or opensuse call it "coin", so I think sticking to names in use is best.

Agreed.

@tonytheodore
Copy link
Member

Files created by package coin:

No *.dll created for shared builds?

The package installs files usr//bin/coin-config which are equal for all targets. Are these files needed? I think, this file should not be installed or be installed to usr/bin/coin-config.

I imagine they use relative paths to locate the installation. We'd normally create a prefixed symlink <target>-coin-config in usr/bin (see gdal or sdl).

@starius
Copy link
Member

starius commented Nov 4, 2015

No *.dll created for shared builds?

By the way, it is a good check for build-pkg.

starius added a commit to LuaAndC/mxe that referenced this pull request Nov 4, 2015
starius added a commit to LuaAndC/mxe that referenced this pull request Nov 5, 2015
@fiesh
Copy link
Author

fiesh commented Nov 5, 2015

I'm trying to add the dynamic library, but I get the following messages that aren't really clear to me:

*** Warning: linker path does not have real file for library -lopengl32.
*** I have the capability to make that library automatically link in when
*** you link to this library.  But I can only do this if you have a
*** shared version of the library, which you do not appear to have
*** because I did check the linker path looking for a file starting
*** with libopengl32 and none of the candidates passed a file format test
*** using a file magic. Last file checked: /tmp/mxe/usr/lib/gcc/x86_64-w64-mingw32.shared/5.2.0/../../../../x86_64-w64-mingw32.shared/lib//libopengl32.a

*** Warning: linker path does not have real file for library -lgdi32.
*** I have the capability to make that library automatically link in when
*** you link to this library.  But I can only do this if you have a
*** shared version of the library, which you do not appear to have
*** because I did check the linker path looking for a file starting
*** with libgdi32 and none of the candidates passed a file format test
*** using a file magic. Last file checked: /tmp/mxe/usr/lib/gcc/x86_64-w64-mingw32.shared/5.2.0/../../../../x86_64-w64-mingw32.shared/lib//libgdi32.a

*** Warning: linker path does not have real file for library -lwinmm.
*** I have the capability to make that library automatically link in when
*** you link to this library.  But I can only do this if you have a
*** shared version of the library, which you do not appear to have
*** because I did check the linker path looking for a file starting
*** with libwinmm and none of the candidates passed a file format test
*** using a file magic. Last file checked: /tmp/mxe/usr/lib/gcc/x86_64-w64-mingw32.shared/5.2.0/../../../../x86_64-w64-mingw32.shared/lib//libwinmm.a
*** The inter-library dependencies that have been dropped here will be
*** automatically added whenever a program is linked with this library
*** or is declared to -dlopen it.

*** Since this library must not contain undefined symbols,
*** because either the platform does not support them or
*** it was explicitly requested with -no-undefined,
*** libtool will only create a static version of it.

Can someone shed some light on how to fix this properly? Thanks!

Add shared / static linking of libraries that doesn't work yet, see #966
(This is correct since the DLLs aren't built correctly.)
Remove link to coin-config since it's broken
@fiesh
Copy link
Author

fiesh commented Nov 5, 2015

OK, so two problems remain:

Firstly, I had to remove the link of coin-config to $(TARGET)-coin-config since coin-config does some PWD trickery to determine its configuration file. This breaks it. Alas also setting $(TARGET)-coin-config up as a separate shell script

#! /bin/sh
$(PREFIX)/$(TARGET)/bin/coin-config

doesn't do the trick. So one would need to patch coin-config, I'm not sure if that's the way to go?

Secondly, the DLLs are still not created as stated above.

I'd be very grateful for any help or recommendations.

@fiesh
Copy link
Author

fiesh commented Nov 9, 2015

No help for me? I don't know how to solve either of the two problems satisfactorily. The first one isn't really that much of an issue I guess. For the second one, I'm not sure what correct behavior would be, given that there are no .dll files for the mentioned libs. So should just the statically linked archive be included in creating the dll?

starius added a commit to LuaAndC/mxe that referenced this pull request Nov 9, 2015
@tonytheodore
Copy link
Member

*** with libopengl32 and none of the candidates passed a file format test
*** using a file magic.

This sounds like you'll have to set the libtool cache variables directly, look at exiv2 or portaudio:

    # libtool looks for a pei* format when linking shared libs
    # apparently there's no real difference b/w pei and pe
    # so we set the libtool cache variables
    # https://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/libpei.h?annotate=1.25&cvsroot=src
    cd '$(1)' && ./configure \
        $(MXE_CONFIGURE_OPTS) \
...
        $(if $(BUILD_SHARED),\
            lt_cv_deplibs_check_method='file_magic file format (pe-i386|pe-x86-64)' \
            lt_cv_file_magic_cmd='$$OBJDUMP -f')

coin-config does some PWD trickery to determine its configuration file. This breaks it.

Not sure what to do about this, most likely will just have to invoke it with the full path or use pkg-config instead.

As an aside, this release looks very old, would it be better to use the latest development version?

@fiesh
Copy link
Author

fiesh commented Nov 10, 2015

@tonytheodore Thanks for the hint, alas, that's not the issue. libtool complains it cannot find the shared libraries to opengl32, winmm, and user32, which is correct since there are none. I don't really understand how to make it create a shared library nonetheless. I wasted a couple hours on it but have no clue what to do, directly supplying the .a files does not help, neither does removing the -no-unneeded as it just makes libtool not go for a shared library at all.

As for the version, I think 3.1.3 is fine. It is what all distributions I checked include, so I think it's good to have the same version that will be available by default. Also, while it's an old version, not that much has happened in the repository either.

@tonytheodore
Copy link
Member

I found a workaround for a similar libtool issue and have rebased and merged this.

We normally enable most features of a package, could you take a look at that and open a new request?

Thanks.

@fiesh
Copy link
Author

fiesh commented Nov 19, 2015

@tonytheodore awesome, thank you so much! I will do that, I thought I read somewhere that the dependencies should be kept minimal, which was why I disabled everything not necessary. But I'll add them!

@tonytheodore
Copy link
Member

No worries, libtool can be a major time sink and we're gradually chipping away at workarounds (partly why I mentioned the newer version that uses cmake - it seems to do a better job with shared libs).

We try to keep native requirements minimal to avoid doc building etc., but otherwise packages should enable as much as possible.

@fiesh
Copy link
Author

fiesh commented Nov 19, 2015

OK, then one more question that isn't clear to me. Since autoconf will enable/disable things automatically based on what it finds, should I just not --disable things and let autoconf and thus the user decide based on what his MXE setup provides, or should the dependencies be listed explicitly, making it actually enable everything it can, but thereby possibly bloating more than the user might want?

Edit: OK, coin actually does run time linking of all its dependencies, so it doesn't matter here. Still it's an interesting question for possible further packages.

@tonytheodore
Copy link
Member

or should the dependencies be listed explicitly, making it actually enable everything it can, but thereby possibly bloating more than the user might want?

Ideally, explicit dependencies and --disable unwanted/broken features. For any given user, it's easier to disable a working feature manually than enable an unknown one, and we'll soon have a way to slim down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants