recp / AssetKit

🎨 Modern 2D/3D - Importer • Exporter • Util - Library, also known as (AssetIO)
Apache License 2.0
185 stars 18 forks source link

Can't build library on linux #27

Closed eLeCtrOssSnake closed 2 years ago

eLeCtrOssSnake commented 2 years ago

I am running Garuda Linux

I git clone the repository run cmake/automake as in the tutorial and i get compile errors after I run make.

Consolidate compiler generated dependencies of target ds
[  0%] Building C object deps/ds/CMakeFiles/ds.dir/src/util.c.o
/home/snake/Desktop/AssetKit/deps/ds/src/util.c: In function ‘ds_print_i64’:
/home/snake/Desktop/AssetKit/deps/ds/src/util.c:122:16: error: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘int64_t’ {aka ‘long int’} [-Werror=format=]
122 |   printf("\t%llu\n", *(int64_t *)key);
|             ~~~^     ~~~~~~~~~~~~~~~
|                |     |
|                |     int64_t {aka long int}
|                long long unsigned int
|             %lu
/home/snake/Desktop/AssetKit/deps/ds/src/util.c: In function ‘ds_print_ui64’:
/home/snake/Desktop/AssetKit/deps/ds/src/util.c:128:16: error: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘uint64_t’ {aka ‘long unsigned int’} [-Werror=format=]
128 |   printf("\t%llu\n", *(uint64_t *)key);
|             ~~~^     ~~~~~~~~~~~~~~~~
|                |     |
|                |     uint64_t {aka long unsigned int}
|                long long unsigned int
|             %lu
cc1: all warnings being treated as errors
make[2]: *** [deps/ds/CMakeFiles/ds.dir/build.make:104: deps/ds/CMakeFiles/ds.dir/src/util.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:672: deps/ds/CMakeFiles/ds.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

If I try to hand fix these errors, a bunch of errors pop up, not just warning, but errors like AK_INLINE, AkFloat3 etc.

eLeCtrOssSnake commented 2 years ago

This what happends when i build using autotools

╭─snake@snake in repo: AssetKit on  master [!?] via △ v3.21.3 took 3s
╰─λ make
(CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh '/home/snake/Desktop/AssetKit/missing' autoheader)
rm -f stamp-h1
touch config.h.in
cd . && /bin/sh ./config.status config.h
config.status: creating config.h
make  all-am
make[1]: Entering directory '/home/snake/Desktop/AssetKit'
CC       src/assetkit.lo
In file included from src/../include/ak/assetkit.h:499,
from src/assetkit.c:17:
src/../include/ak/transform.h:33:3: error: size of array element is not a multiple of its alignment
33 |   AkFloat3     val[3];
|   ^~~~~~~~
In file included from src/io/gltf/common.h:23,
from src/io/gltf/gltf.h:20,
from src/assetkit.c:21:
src/io/gltf/../../tree.h:28:14: warning: ‘visibility’ attribute ignored on non-class types [-Wattributes]
28 |              xml_t  * __restrict xml);
|              ^~~~~
In file included from src/io/stl/stl.h:20,
from src/assetkit.c:23:
src/io/stl/common.h:42: warning: "SKIP_SPACES" redefined
42 | #define SKIP_SPACES                                                           \
| 
In file included from src/io/obj/obj.h:20,
from src/assetkit.c:22:
src/io/obj/common.h:81: note: this is the location of the previous definition
81 | #define SKIP_SPACES                                                           \
| 
make[1]: *** [makefile:1826: src/assetkit.lo] Error 1
make[1]: Leaving directory '/home/snake/Desktop/AssetKit'
make: *** [makefile:971: all] Error 2
eLeCtrOssSnake commented 2 years ago

I am building with gcc (GCC) 11.1.0

recp commented 2 years ago

Hello @eLeCtrOssSnake ,

I was working on morph targets, I'm updating the design; I'll check build file asap ;)

eLeCtrOssSnake commented 2 years ago

Thank you for your reply @recp. I am looking forward to use this library, because assimp didn't really work well, mostly because it's C++(as I am writing in C) and it is very slow at linking process when I am building my project. But this seems like a solid choice.

I will be checking up on this quite frequently so you can ask me for help if needed.

recp commented 2 years ago

I will be checking up on this quite frequently so you can ask me for help if needed.

I'll try to be fast as possible then 🤗

AssetKit will try to be fast as possible and try to support all formats' features as it can be with single interface.

I'll try my best to complete unfinished parts, including build, I'll let you know here.

eLeCtrOssSnake commented 2 years ago

Any updates?

eLeCtrOssSnake commented 2 years ago

Any updates?

recp commented 2 years ago

@eLeCtrOssSnake sorry for the late response,

I still need more time (I'm busy for a while) to finish morph targets, material system may need to be updated for better design but existing one can be used for now.

Currently I'm tryig to fix build errors and will let you know (in a few hours I hope) to let you give a try AssetKit with existing codebase. Maybe getting feedbacks from you and community will help to fix things faster...

recp commented 2 years ago

@eLeCtrOssSnake can you try again pls? There are compiler warnings, I'll try to fix them asap...

eLeCtrOssSnake commented 2 years ago

Okay, let's see if it will build. Trying out now.

recp commented 2 years ago

Your compiler seems to treat warings as errors, I'll try to fix/suppress all warnings, asap.

eLeCtrOssSnake commented 2 years ago

Can't generate cmake build files.

CMake Error at CMakeLists.txt:62 (target_compile_definitions):
  Cannot specify compile definitions for target "assetkit" which is not built
  by this project.

Submodule update
CMake Warning at /usr/share/cmake-3.21/Modules/CPack.cmake:467 (message):
  CPack.cmake has already been included!!
Call Stack (most recent call first):
  deps/ds/CMakeLists.txt:42 (include)
eLeCtrOssSnake commented 2 years ago

Your compiler seems to treat warings as errors, I'll try to fix/suppress all warnings, asap.

I tried making compiler ignore warning in my first attempt, but it did throw errors afterwards.

recp commented 2 years ago

My main environment is macOS / Xcode / clang (not gcc) this iswhy I'm not getting warnings/errors...

Currently I'm trying to fix errors/warnings on virtual Fedora (gcc 11) and Ubuntu (gcc 9.3) annd Widnows (msvc). I'll let you know when they are fixed.

recp commented 2 years ago

but it did throw errors afterwards.

What kind of errors may I ask? Like alignment errors which I get from Fedora (gcc11) now?

eLeCtrOssSnake commented 2 years ago

I had something like float typedef?? thing? I can't remember. I'd show but I can't generate cmake files to build.

eLeCtrOssSnake commented 2 years ago

Okay so setting AK_STATIC instead of AK_SHARED prevents me from cmake from generating build files.

eLeCtrOssSnake commented 2 years ago

This is what I get now: Make log Cmake config

recp commented 2 years ago

Can you pull again pls? I guess build should work now with few warnings

eLeCtrOssSnake commented 2 years ago

I have successfully built assetkit and ds in shared mode, but I cannot get them to build in static mode. When I choose AK_STATIC it fails to generate:

CMake Error at CMakeLists.txt:62 (target_compile_definitions):
  Cannot specify compile definitions for target "assetkit" which is not built
  by this project.
eLeCtrOssSnake commented 2 years ago

I'd do something with cmake because it looks like it's something easy to fix, but I have 0 knowledge in cmake.

recp commented 2 years ago

@eLeCtrOssSnake all warnings are fixed or suppressed except the one below 🚀:

Fedora (gcc 11) / Ubuntu (gcc 9.3) [ Autotools and Cmake builds ]

src/coord/transforms.c: In function ‘ak_coordCvtNodeTransforms’:
src/coord/transforms.c:97:17: warning: variable ‘skew’ set but not used [-Wunused-but-set-variable]
   97 |         AkSkew *skew;
      |                 ^~~~

because Skew transform is not implemented yet.

I'll check windows build too

eLeCtrOssSnake commented 2 years ago

Does the static build work @recp? For me it doesn't.

recp commented 2 years ago

@eLeCtrOssSnake can you pull and try again pls? I have updated CMake file.

Default must be shared and static is an option.

mkdir build && cd build

cmake .. # same as below
cmake .. -DAK_SHARED=ON

# or
cmake .. -DAK_STATIC=ON

make -j8
eLeCtrOssSnake commented 2 years ago

Can't seem to build it with mingw on windows. Should be pretty close to linux, as I don't wanna boot over on over again. Same compiler at least. Can you check that @recp?

Consolidate compiler generated dependencies of target ds
[  0%] Building C object deps/ds/CMakeFiles/ds.dir/src/default/default.c.obj
F:\AssetKit\deps\ds\src\default\default.c: In function 'ds_def_alc':
F:\AssetKit\deps\ds\src\default\default.c:28:1: error: visibility attribute not supported in this configuration; ignored [-Werror=attributes]
   28 | }
      | ^
cc1.exe: all warnings being treated as errors
deps\ds\CMakeFiles\ds.dir\build.make:75: recipe for target 'deps/ds/CMakeFiles/ds.dir/src/default/default.c.obj' failed
mingw32-make[2]: *** [deps/ds/CMakeFiles/ds.dir/src/default/default.c.obj] Error 1
CMakeFiles\Makefile2:688: recipe for target 'deps/ds/CMakeFiles/ds.dir/all' failed
mingw32-make[1]: *** [deps/ds/CMakeFiles/ds.dir/all] Error 2
Makefile:154: recipe for target 'all' failed
mingw32-make: *** [all] Error 2
recp commented 2 years ago

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14654

The reason why it is not supported is because your binutils does not support it, update that and rebuild GCC and you will get the attribute visibility working.

It seems build is not an issue anymore, I'm going to close this issue soon.

eLeCtrOssSnake commented 2 years ago

hm it might be a problem in mingw

recp commented 2 years ago

Also you can build AssetKit with MSBuild / Visual Studio or Autotools (if Linux subsystem is installed: https://docs.microsoft.com/en-us/windows/wsl/install)

eLeCtrOssSnake commented 2 years ago

I don't wanna go back to msvc, it's such a mess. Gonna try newest MinGW and see if there's a difference. Probs I am doing something wrong now.

recp commented 2 years ago

Gonna try newest MinGW and see if there's a difference

Great!

eLeCtrOssSnake commented 2 years ago

The latest mingw prebuilts aren't much newer, same error. Pretty late now for me. Gonna do complete testing tomorrow with linux and windows so it will be 100% clear.

recp commented 2 years ago

@eLeCtrOssSnake thanks, I may try to install MinGW too later and try to see what is wrong

recp commented 2 years ago

Maybe we can disable visibility attribute (AK_HIDE and AK_EXPORT) if it is not supported to ignore this:

 error: visibility attribute not supported in this configuration; ignored

added to TODOs...

eLeCtrOssSnake commented 2 years ago

Ye, the reason why I am using gcc even on windows is because it's easy to work with and totally transparent. And it's cross-platform so I don't have to complicate a ton of stuff just to get it working on other systems.

I forgot to check mingw(tho I run tdm-gcc because it at least has colorful console output, like rly? nice port there mingw) version that I had, but I believe it was like 10 or 11. That's quite new. Not very sure about it though.


From: Recep Aslantas @.***> Sent: Thursday, October 28, 2021, 11:42 PM To: recp/AssetKit Cc: eLeCtrOssSnake; Mention Subject: Re: [recp/AssetKit] Can't build library on linux (#27)

@eLeCtrOssSnakehttps://github.com/eLeCtrOssSnake thanks, I may try to install MinGW too later and try to see what is wrong

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/recp/AssetKit/issues/27#issuecomment-954197285, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJSXC4QV3FNJQ5UCEZUKI6DUJGYS5ANCNFSM5E3PSJ3Q.

eLeCtrOssSnake commented 2 years ago

Hmm I ran tdm-gcc which is 10.3.0 and mingw which was 8.1.0 both complained about the attribute. It's strange that it complains about their support even though it's being built in static mode, which shouldn't have any visibility attributes. Gonna try building on linux now.

eLeCtrOssSnake commented 2 years ago

Okay, so the problem is that you assume if _MSC_VER is not defined that it's linux. That's what prevents it from building on windows under mingw because mingw obviously uses WinAPI(although there is msys but it's hacky, why if we already have WinAPI versions). So it would be better to check against linux define rather then msvc compiler version. There's only one place where it warns about attribute being incorrect due to config is in ds_def_alc(). If you remove that attribute (and choose correct attributes ones for mingw and not dllimport) then it will build perfectly fine.

eLeCtrOssSnake commented 2 years ago

Successfully builds on linux.

recp commented 2 years ago

Okay, so the problem is that you assume if _MSC_VER is not defined that it's linux. That's what prevents it from building on windows under mingw because mingw obviously uses WinAPI(although there is msys but it's hacky, why if we already have WinAPI versions). So it would be better to check against linux define rather then msvc compiler version. There's only one place where it warns about attribute being incorrect due to config is in ds_def_alc(). If you remove that attribute (and choose correct attributes ones for mingw and not dllimport) then it will build perfectly fine.

Thanks for that. I'll try to fix testing against msvc/mingw (_MSC_VER) later if this is the problem but any help would be appreciated ;)

Successfully builds on linux.

Great! Sice the library is new bugs may occour; I'll try to fix them (letting me know will make the process quicker I guess) and update documentation asap/time by time...

recp commented 2 years ago

Just created a PR: https://github.com/recp/AssetKit/pull/29

@eLeCtrOssSnake can you review and re-build the PR on mingw?

Reference: https://stackoverflow.com/a/9939664/2676533

eLeCtrOssSnake commented 2 years ago

Ye, on it now @recp.

recp commented 2 years ago

I'm not sure if the attributes work on mingw too:

#  define AK_ALIGN(X) __attribute((aligned(X)))
#  define AK_INLINE   static inline __attribute((always_inline))

it may expect msvc style (__forceinline, __declspec(align(X))) instead of gcc I don't know.

eLeCtrOssSnake commented 2 years ago

I'll check it, ye it's kinda counter-intuitive.

recp commented 2 years ago

Many thanks

eLeCtrOssSnake commented 2 years ago

I am pretty confident to say that MinGW supports both forceinline(defined as `extern inline attribute((alwaysinline, __gnu_inline))` and attribute((aligned(X))), but I am pretty sure MinGW supports msvc style atributes as it has it's own defs as gcc style attributes.

recp commented 2 years ago

I'm confused about what to do, but it seems we can leave these attributes as they are.

Did build successful on mingw with that PR?

recp commented 2 years ago

Resolved at: https://github.com/recp/AssetKit/pull/29

@eLeCtrOssSnake many thanks...