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
CI: use AppVeyor for testing Windows build, upload artifacts from both Travis and AppVeyor #358
Conversation
.appveyor.yml
Outdated
- BUILD_TYPE: amd64_mingw_static | ||
MINGW_PATH: C:\msys64\mingw64\bin | ||
CMAKE_FLAGS: -G "MinGW Makefiles" -DBUILD_SHARED_LIBS=OFF | ||
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you use 2015 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andlabs should we use 2015 everywhere? Since it is the runtime included in windows-build-tools, it would require one less runtime library to inistall
for libui-node users. Actually, both 2013 and 2015 runtimes are required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... I just always use 2015 in my projects. But yes, should work on 2013 and 2017 too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add any configuration here, but building every configuration takes around one minute.
And for free appveyor does not enable concurrent builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just pick what @andlabs prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compilerver.hpp has my minimum versions. Testing along the matrix of every version starting from it would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added 2015 and 2017.
14 builds will run 20-25 minutes...
https://ci.appveyor.com/project/msink/libui/build/build%20%2321
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, actually it was 32 minutes.
Did you test the example exes? |
No, this is only first step - test buildability. |
I notice the static version lib is huge, why? |
CMakeLists.txt
Outdated
@@ -81,7 +81,6 @@ if(MSVC) | |||
set(_COMMON_CFLAGS | |||
/W4 /wd4100 | |||
/bigobj /nologo | |||
/RTC1 /RTCs /RTCu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this line? With this options enabled all MSVC Release builds failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a separate PR I need to merge at some point.
I would put the shared libraries before the static libraries because they do not require any extra cmake arguments. So are we just abandoning @parro-it's work then? |
So what exactly order you want? And maybe add About #157 - I actually take some pieces from it, so at least for now it needed. |
Will try with Travis too, tomorrow. But @parro-it - I see in your PR patches for building ia32 - do you really need ia32 or you confused it with i686? It not the same. |
Do the Itanium versions of Windows Vista and Server 2008 have the Platform Update, and does the Itanium version of 2008 R2 have Direct2D (and possibly in the future, its Platform Update)? If so, then libui for Windows could run on it, but I wouldn't be able to test it... Would be cool to see running, though. The order I'm imagining is something like
but that depends on how the AppVeyor matrix feature works... |
Awesome!
I confused it |
@andlabs Sorry, but I still do not understand what build sequence you want :) I think in our case it should be simple flat one-dimension matrix. Well, maybe add second dimension: So you want this build sequence:
Ot this:
Or something else? And what resulting artifacts you and @parro-it want? Something like this loooong list:
Not that it's major issue - you always can reshuffle/rename anything youself - but still. |
Okay, so I'm not familiar with things. What is this AppVeyor matrix feature? The order I was thinking of was
And I'm not sure what artifacts, hm... That would depend on how far a single artifact can be used. MinGW is a problem because of exception handling models, and I'm not sure what DLL artifacts can be used by which versions of MSVC. If any of you have answers to these questions, I'd love to hear them. Exception handling model is a bug at andlabs/ui#281. (I should probably sign up for AppVeyor at some point before merging this or the other one.) |
Updated Travis build, but now
Don't know what is |
liblibui-temporary and liblibui-combined is what I'm trying to get rid of in #308. It's now gone on macOS; GTK+ to come later. The problem is I intuited what artifacts are; I was asking what AppVeyor's matrix feature is. And I still wouldn't know what artifacts we actually need to provide, which was the question I thought you were asking earlier =P And all right at merging the PRs; thanks for working on this to the both of you! Another thing I wonder; I forgot if I asked this in the other PR — does Travis provide anything that AppVeyor does not? I'm only using Travis now because it was the first one someone recommended to me (in andlabs/ui#3). |
😎 great work @msink!
The list is from last published release, that I tagged "alpha3.5-master-002".
There's really something there that you don't already have here? |
So for now I commented out 'linux_386_static' build until linking will be fixed, Travis and AppVeyor are almost identical, but for linux+ios and windows respectively. So no competition, nice coexistance. @parro-it I personally do not like |
ia32 was what I get in Node.js as architecture on an old laptop I was using in 2016. Other possible values for node.js architecture are 'arm', 'arm64', 'ia32', 'mips', 'mipsel', 'ppc', 'ppc64', 's390', 's390x', 'x32', and 'x64'. Anyway, if you are strong on using other names for the arch that you felt more appropriate that's ok for me, I'll just change libui-download code to map the arch name I get from the OS to the libui name. |
@parro-it Ok, maybe it's me confused somewhat. Anyway, not a major issue :) |
@andlabs Sorry but I cannot say much about matrix details on travis and appveyor. |
I prefer Go's architecture naming: And yes, IA-32 is another name for the standard 386 ISA. IA-64 is Itanium. |
AFAIK Intel 386 ISA on Linux have two different incompatible ABI's - i386 (old) and x32 (new, since kernel 3.4). |
BTW, dirty hack for make diff --git a/unix/CMakeLists.txt b/unix/CMakeLists.txt
index eba09ad..1ec9b88 100644
--- a/unix/CMakeLists.txt
+++ b/unix/CMakeLists.txt
@@ -67,7 +67,7 @@ macro(_handle_static)
add_custom_command(
OUTPUT ${_oname}
COMMAND
- ld -r --whole-archive ${_aname} -o ${_oname}
+ ld -m elf_i386 -r --whole-archive ${_aname} -o ${_oname}
COMMAND
objcopy --localize-hidden ${_oname}
COMMENT "Removing hidden symbols") |
Hopefully the static build will actually work once I get rid of the temporary and combined madness. If that's not the case, then well... :\ In fact I'm just going to go and declare that merging this will be blocked on #308 being resolved. Keep working on it in the meantime; I just won't merge anything until that one is resolved. It should get rid of most of the building pains. |
9255c1e
to
4c56435
Compare
Ok, all blocking issues fixed, ready to merge. As starting point. |
.appveyor.yml
Outdated
|
||
after_build: | ||
- dir /S build\out | ||
- if %platform%==x64 ( set "platform=amd64" ) else ( set "platform=386" ) | ||
- echo set "target=%linking%-windows-%compiler%-%platform%" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with the prior releases, I'd have the order be windows-%platform%-shared
and windows-%platform%-%compiler%-static
.
Also %platform%
would probbaly make more sense if it was named %arch%
instead.
Or does AppVeyor require these particular formats? Either way, I can change these myself if you want; I'll set up AppVeyor sometime today (if not tonight).
.travis.yml
Outdated
matrix: | ||
include: | ||
- env: | ||
- BUILD_TYPE: shared-linux-amd64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yes, for consistency with prior releases, this should be linux-amd64-shared
(and likewise for the rest), but again, I'm not sure if github or something else is imposing a requirement on these names. Are they?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are they?
No, you can call them as you prefer...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already refactored this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, sorry - only appveyor was refactored for now.
Hm, not sure. |
Also I would personally have the libs and examples in the same archive too, but I can do this myself. So the important question: how would I trigger artifact deployment here? I only need artifacts deployed when I tag a version. |
Last two lines of For now I finished with appveyor, uploading from travis is missed. |
For sure, but having two separate formats force me to add another dependency to libui-download to handle zip format, and to process the files in two different ways depending on the platform.
This also is not desirable for me, having the example in the same archive as the libraries A compromise could be to have some archive published just for the sake of bindings implementors, containing only library and headers, and all having the same format. |
Wat, so you are not shipping libui with libui-node? I can't think of any issue with that, but it does seem very unusual... do any of the other libui bindings do this? |
Not at all, or I would have to include all the platform binaries in the package... at the time I start, counting the 32bit ver, that would have been 6 binaries to include, just to pick the one efffectively used in a machine. |
@parro-it - maybe better way is just compile libui on travis+appveyor as I did in my simple Kotlin/Native bindings https://github.com/msink/klib-ui ? |
@msink you mean, without compression? It's not a way, I need also the libui headers at install time to compile libui-node... |
@parro-it Why, you can pack and compress what In my case Kotlin does it by builtin |
Yes, I can keep my fork alive and just publish the binaries there the way I need to. I just think that using "official" binaries published in the main repos would be nice. |
It depends - as libui is currently alpha, I personally prefer to be on "bleeding edge" - releases are rare, and some bugfixes are critical. |
I completely agree. So I'll just continue to publish from my fork for now... |
Ok, Travis works too. |
Excellent, thanks! I'll worry about the artifact naming myself, I suppose. Shouldn't be too hard to fix. I wonder if it would be safe to remove your API keys from the files before merging, lest either of us get yelled at by either AppVeyor or Travis's security team?... |
Keys will be checked only when/if you try set |
Please update readme.md to show appveory status. |
No description provided.