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

CI: use AppVeyor for testing Windows build, upload artifacts from both Travis and AppVeyor #358

Merged
merged 6 commits into from May 17, 2018

Conversation

msink
Copy link
Contributor

@msink msink commented May 4, 2018

No description provided.

.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
Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

@msink msink May 4, 2018

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.

Copy link
Contributor

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.

Copy link
Owner

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

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.

@parro-it
Copy link
Contributor

parro-it commented May 4, 2018

Did you test the example exes?

@msink
Copy link
Contributor Author

msink commented May 4, 2018

No, this is only first step - test buildability.
Next step - zipping artifacts.
I just do not know is it valid at all, let's wait for @andlabs

@parro-it
Copy link
Contributor

parro-it commented May 4, 2018

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
Copy link
Contributor Author

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

Copy link
Owner

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.

@andlabs
Copy link
Owner

andlabs commented May 4, 2018

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?

@msink
Copy link
Contributor Author

msink commented May 4, 2018

So what exactly order you want? And maybe add Debug builds? It will be an hour...

About #157 - I actually take some pieces from it, so at least for now it needed.

@parro-it
Copy link
Contributor

parro-it commented May 4, 2018

So are we just abandoning @parro-it's work then?

No worries, using the Matrix functionality seems better.
@msink do you know if Travis has similar features?
Would you mind to also take care of that part?

@msink
Copy link
Contributor Author

msink commented May 4, 2018

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.

@andlabs
Copy link
Owner

andlabs commented May 4, 2018

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

VS
  2013
    x86
      shared
      static
    x64
  2015
MinGW-w64
  (possibly other matrix axes here, IDK)

but that depends on how the AppVeyor matrix feature works...

@parro-it
Copy link
Contributor

parro-it commented May 4, 2018

Will try with Travis too, tomorrow.

Awesome!

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.

I confused it

@msink
Copy link
Contributor Author

msink commented May 5, 2018

@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.
Because every build type requires different CMAKE_FLAGS, and better keep it explicit in matrix, then build script is trivial and obvious.

Well, maybe add second dimension: Debug/Release, but I think Debug builds should not run every push/pull request, on tagged release - maybe, don't know.

So you want this build sequence:

x86_msvc2013_shared
x86_msvc2013_static
x86_msvc2015_shared
x86_msvc2015_static
x86_msvc2017_shared
x86_msvc2017_static
x64_msvc2013_static
x64_msvc2013_shared
x64_msvc2015_static
x64_msvc2015_shared
x64_msvc2017_static
x64_msvc2017_shared
x86_mingw_static
x64_mingw_static

Ot this:

x64_msvc2013_static
x64_msvc2013_shared
x86_msvc2013_shared
x86_msvc2013_static
x64_msvc2015_static
x64_msvc2015_shared
x86_msvc2015_shared
x86_msvc2015_static
x64_msvc2017_static
x64_msvc2017_shared
x86_msvc2017_shared
x86_msvc2017_static
x64_mingw_static
x86_mingw_static

Or something else?

And what resulting artifacts you and @parro-it want? Something like this loooong list:

windows_x86_msvc2013_shared_debug.zip
windows_x86_msvc2013_shared_release.zip
windows_x86_msvc2013_shared_examples.zip
windows_x86_msvc2013_static_debug.zip
windows_x86_msvc2013_static_release.zip
windows_x86_msvc2013_static_examples.zip
windows_x86_msvc2015_shared_debug.zip
windows_x86_msvc2015_shared_release.zip
windows_x86_msvc2015_shared_examples.zip
windows_x86_msvc2015_static_debug.zip
windows_x86_msvc2015_static_release.zip
windows_x86_msvc2015_static_examples.zip
windows_x86_msvc2017_shared_debug.zip
windows_x86_msvc2017_shared_release.zip
windows_x86_msvc2017_shared_examples.zip
windows_x86_msvc2017_static_debug.zip
windows_x86_msvc2017_static_release.zip
windows_x86_msvc2017_static_examples.zip
windows_x86_mingw_static_debug.zip
windows_x86_mingw_static_release.zip
windows_x86_mingw_static_examples.zip   
windows_x64_msvc2013_static_debug.zip
windows_x64_msvc2013_static_release.zip
windows_x64_msvc2013_static_examples.zip
windows_x64_msvc2013_shared_debug.zip
windows_x64_msvc2013_shared_release.zip
windows_x64_msvc2013_shared_examples.zip
windows_x64_msvc2015_static_debug.zip
windows_x64_msvc2015_static_release.zip
windows_x64_msvc2015_static_examples.zip
windows_x64_msvc2015_shared_debug.zip
windows_x64_msvc2015_shared_release.zip
windows_x64_msvc2015_shared_examples.zip
windows_x64_msvc2017_static_debug.zip
windows_x64_msvc2017_static_release.zip
windows_x64_msvc2017_static_examples.zip
windows_x64_msvc2017_shared_debug.zip
windows_x64_msvc2017_shared_release.zip
windows_x64_msvc2017_shared_examples.zip
windows_x64_mingw_static_debug.zip
windows_x64_mingw_static_release.zip
windows_x64_mingw_static_examples.zip 

Not that it's major issue - you always can reshuffle/rename anything youself - but still.

@andlabs
Copy link
Owner

andlabs commented May 5, 2018

Okay, so I'm not familiar with things. What is this AppVeyor matrix feature?

The order I was thinking of was

x86_msvc2013_shared
x86_msvc2013_static
x64_msvc2013_shared
x64_msvc2013_static
x86_msvc2015_shared
x86_msvc2015_static
x64_msvc2015_shared
x64_msvc2015_static
x86_msvc2017_shared
x86_msvc2017_static
x64_msvc2017_shared
x64_msvc2017_static
x86_mingw_static
x64_mingw_static

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.)

@msink
Copy link
Contributor Author

msink commented May 6, 2018

Updated Travis build, but now linux_386_static failed:

[ 71%] Linking C static library liblibui-temporary.a
[ 71%] Built target libui-temporary
[ 72%] Removing hidden symbols
ld: Relocatable linking with relocations from format elf32-i386 (/home/travis/build/andlabs/libui/build/liblibui-temporary.a(attribute.c.o)) to format elf64-x86-64 (libui-combined.o) is not supported

Don't know what is liblibui-temporary and liblibui-combined, and why all that tampering needed.

@msink
Copy link
Contributor Author

msink commented May 7, 2018

@andlabs In this context artifact is part of distribution, basically what @parro-it wants in #157 - split single file alpha3.5.tgz into multiple artifacts.

About mingw - well, it's really `msys2-mingw', maybe name is too generic.

@msink
Copy link
Contributor Author

msink commented May 7, 2018

My plan is to somewhat rebase #157 on top ow this - it will become order of magnitude smaller and cleaner.
If @parro-it have time and willingness - he can do, if not - I can do myself, but later, when/if this will be merged.

@andlabs
Copy link
Owner

andlabs commented May 7, 2018

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 -fvisibility-hidden didn't apply to static libraries, but re-linking to remove hidden symbols fixed it; doing this was just too quirky and had other problems as well.

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).

@parro-it
Copy link
Contributor

parro-it commented May 7, 2018

😎 great work @msink!
Regarding libui-node needs, I just use these artifacts:

libui-shared-linux-ia32-alpha3.5-master-002.tar.gz
libui-shared-linux-x64-alpha3.5-master-002.tar.gz
libui-shared-osx-ia32-alpha3.5-master-002.tar.gz
libui-shared-osx-x64-alpha3.5-master-002.tar.gz
libui-shared-windows-ia32-alpha3.5-master-002.tar.gz
libui-shared-windows-x64-alpha3.5-master-002.tar.gz

The list is from last published release, that I tagged "alpha3.5-master-002".
It would be beautiful if you can keep the same naming convention, that would preserve me to change libui-download to use the new names.

My plan is to somewhat rebase #157 on top ow this - it will become order of magnitude smaller and cleaner.

There's really something there that you don't already have here?

@msink
Copy link
Contributor Author

msink commented May 7, 2018

So for now I commented out 'linux_386_static' build until linking will be fixed,
What to do with MSVC /RTC1 /RTCs /RTCu options - don't know.

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/x64 naming, it confusing prone.
IMHO it should be x86/x86_64 or i386/x86_64

@parro-it
Copy link
Contributor

parro-it commented May 7, 2018

@parro-it I personally do not like ia32/x64 naming, it confusing prone.

ia32 was what I get in Node.js as architecture on an old laptop I was using in 2016.
I can't swear it really was an ia32, but the libui build was running smoothly there...

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.

@msink
Copy link
Contributor Author

msink commented May 7, 2018

@parro-it Ok, maybe it's me confused somewhat. Anyway, not a major issue :)

@msink
Copy link
Contributor Author

msink commented May 7, 2018

@andlabs Sorry but I cannot say much about matrix details on travis and appveyor.
Definitely they are a little different in details - different implementations of the same idea.
And both evolve in same direction, influencing each other.

@andlabs
Copy link
Owner

andlabs commented May 7, 2018

I prefer Go's architecture naming: 386 and amd64. I already use it in the binary releases I make by hand, and would rather it stay with this.

And yes, IA-32 is another name for the standard 386 ISA. IA-64 is Itanium.

@msink
Copy link
Contributor Author

msink commented May 7, 2018

AFAIK Intel 386 ISA on Linux have two different incompatible ABI's - i386 (old) and x32 (new, since kernel 3.4).
So I too prefer 386 over ia32 - less error prone.

@msink
Copy link
Contributor Author

msink commented May 7, 2018

BTW, dirty hack for make linux_386_static buildable:

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")

@andlabs
Copy link
Owner

andlabs commented May 7, 2018

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.

@msink
Copy link
Contributor Author

msink commented May 14, 2018

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%"
Copy link
Owner

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
Copy link
Owner

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?

Copy link
Contributor

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...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already refactored this :)

Copy link
Contributor Author

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.

@parro-it
Copy link
Contributor

@andlabs, @msink it would be useful if artifacts could be compressed using .tar.gz format, as it is what I'm using now in automated download of binaries during libui-node installation.

@msink
Copy link
Contributor Author

msink commented May 15, 2018

Hm, not sure.
.tar.gz is not so common on Windows as .zip
But can do, let @andlabs decide.

@andlabs
Copy link
Owner

andlabs commented May 15, 2018

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.

@msink
Copy link
Contributor Author

msink commented May 15, 2018

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 .appveyor.yml are just for that.
You can do git tag and push locally, or just create release in GitHub web interface - anyway, if appveyor sees new tag, it will upload and attach all artefacts into release.
I personally prefer do it in web inteface.

For now I finished with appveyor, uploading from travis is missed.

@parro-it
Copy link
Contributor

.tar.gz is not so common on Windows as .zip

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.

Also I would personally have the libs and examples in the same archive too, but I can do this myself.

This also is not desirable for me, having the example in the same archive as the libraries
increment the size of the download when user install libui-node, and I'll have to manually delete the example after the download.

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.
For my needs, I'll just use the artifacts for shared libraries, 64bit of windows, linux and macOS

@andlabs
Copy link
Owner

andlabs commented May 15, 2018

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?

@parro-it
Copy link
Contributor

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.
This is a common pattern on npm.

@msink
Copy link
Contributor Author

msink commented May 15, 2018

@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 ?

@parro-it
Copy link
Contributor

@msink you mean, without compression? It's not a way, I need also the libui headers at install time to compile libui-node...

@msink
Copy link
Contributor Author

msink commented May 16, 2018

@parro-it Why, you can pack and compress what node want, and upload to your GitHub release as separate artefact. Just a little more scripting.

In my case Kotlin does it by builtin cinterop tool, producing single self-contained file *.klib

@parro-it
Copy link
Contributor

@parro-it Why, you can pack and compress what node want, and upload to your GitHub release as separate artefact. Just a little more scripting.

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.

@msink
Copy link
Contributor Author

msink commented May 16, 2018

It depends - as libui is currently alpha, I personally prefer to be on "bleeding edge" - releases are rare, and some bugfixes are critical.

@parro-it
Copy link
Contributor

I completely agree. So I'll just continue to publish from my fork for now...
We can discuss again when 1.0.0 will come 😁

@msink
Copy link
Contributor Author

msink commented May 16, 2018

Ok, Travis works too.
https://github.com/msink/libui/releases/tag/test-2

@msink msink changed the title CI: use AppVeyor for testing Windows buildability CI: use AppVeyor for testing Windows build, upload artifacts from both Travis and AppVeyor May 16, 2018
@andlabs
Copy link
Owner

andlabs commented May 16, 2018

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?...

@msink
Copy link
Contributor Author

msink commented May 16, 2018

Keys will be checked only when/if you try set git tag for release, for git push they will be just ignored.
So I think merging this is safe.

@andlabs andlabs merged commit 4245120 into andlabs:master May 17, 2018
@zhaozg
Copy link
Contributor

zhaozg commented May 17, 2018

Please update readme.md to show appveory status.

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