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 .gitlab-ci.yml #2345

Merged
merged 5 commits into from
Mar 31, 2017
Merged

add .gitlab-ci.yml #2345

merged 5 commits into from
Mar 31, 2017

Conversation

skwerlman
Copy link
Contributor

This adds a configuration file for GitLab CI.

This commit is necessary so that binaries compiled on my GitLab instance have the same commit hash as those compiled on Travis.

I will be compiling binaries for any OS where there is a docker image and no TravisCI support. (Currently Ubuntu 16.04 & 16.10)

@tooomm
Copy link
Member

tooomm commented Jan 5, 2017

Sounds awesome! 👍

@Daenyth
Copy link
Member

Daenyth commented Jan 5, 2017

Can gitlab upload to bintray? I'd rather just bring this internal. I don't want to promote anyone else's builds because users don't have a good way to trust the contents. We don't have signed builds set up yet but it is on the roadmap

@tooomm
Copy link
Member

tooomm commented Jan 5, 2017

Can we upload his build to our releases at least for the moment? Releases are manually uploaded anyway.
To offer support for recent ubuntu lts versions in the meantime...

We could include ubuntu builds for ourselves, Travis can handle docker container as well.
In another ticket we talked about that already and I think there is more information in some travis issues on their github. Can you look into that instead @skwerlman?

@woogerboy21
Copy link
Contributor

So whats the difference between gitlab and github?

@skwerlman
Copy link
Contributor Author

@Daenyth Yes. I've had it do that for a couple of my other projects. It's as simple as calling the API command via curl.

@tooomm I could, but I'm much less familiar w/ Travis, and I already have GitLab CI fully working. Plus, almost none of the work I've done to get it working is transferable to Travis.

@woogerboy21 The main difference is that GitLab can be self-hosted, like mine is. It also has its own self-contained CI system, and has unlimited private repos.

@skwerlman
Copy link
Contributor Author

(don't merge this yet; i'm making a few improvements which will make it easier to add automatic uploads and non-debian oses later)

@ZeldaZach
Copy link
Member

ZeldaZach commented Jan 14, 2017

I'm not sure what to do with this PR. I'm hesitant to start using another service until we all talk about it as a team.

@woogerboy21
Copy link
Contributor

Eh, it's really up to you guys. I have a personal gitlab server in house that I use for smaller projects and copies of repo's from github so I can go either way. By no means am I an expert with it.

@ctrlaltca
Copy link
Contributor

Looks like this currently depends on #2351.

@Daenyth
Copy link
Member

Daenyth commented Jan 18, 2017

Is there hosted gitlab CI we can use? I'm not interested in hosting it myself and I'd like a service just so I can trust their security rather than a one-person owned server somewhere

@skwerlman
Copy link
Contributor Author

skwerlman commented Jan 18, 2017

there is, yes: https://gitlab.com (you have to sign up before it shows the gitlab ui)

@ZeldaZach
Copy link
Member

I saw gitlab had issues a few weeks ago and I'd rather avoid them for now, especially if we're using their hosting services...

.gitlab-ci.yml Outdated
script:
- mkdir build || true # allowed to fail
- cd build
- cmake .. -DWITH_SERVER=1 -DCMAKE_BUILD_TYPE=Release -DPACKAGE_GENERATOR=DEB
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update this to use -DCPACK_GENERATOR instead of -DPACKAGE_GENERATOR as implemented in #2373? Thank you

@ctrlaltca
Copy link
Contributor

I think it's would be nice to get this merged in anyway. It's a step forward towards providing linux packages for any recent distro.

@skwerlman
Copy link
Contributor Author

(there are still a few minor changes I'd like to make before this gets merged)

@ZeldaZach
Copy link
Member

What changes are still necessary here?

@skwerlman
Copy link
Contributor Author

There are still a few problems with this:

  • All debug builds are failing for an unknown reason (it only happens when built with -DCMAKE_BUILD_TYPE=Debug) (I think it'll be easier to drop debug builds here)
  • The debian stretch build is failing because it can't find QWebSocketServer (easy fix)
  • ctrlaltca requested changes to be consistent with add RPM support to make package #2373

I'd like to get these done before merging. I'll be able to work on this again today, so hopefully it won't take too long.

@Daenyth
Copy link
Member

Daenyth commented Mar 16, 2017

All debug builds are failing for an unknown reason

Is there an error message? The debug builds have more strict cflags and also provide the unit tests

@skwerlman
Copy link
Contributor Author

skwerlman commented Mar 16, 2017

$ cmake .. -DWITH_SERVER=1 -DCMAKE_BUILD_TYPE=Debug -DPACKAGE_GENERATOR=DEB
-- Found Git: /usr/bin/git (found version "2.9.3") 
-- Performing Test CXX_HAS_WARNING_-Wno-error=extra
CMake Error: Generator: execution of make failed. Make command was: "/usr/bin/gmake" "cmTC_2ded4/fast"
-- Performing Test CXX_HAS_WARNING_-Wno-error=extra - Failed
-- Performing Test CXX_HAS_WARNING_-Wno-error=delete-non-virtual-dtor
CMake Error: Generator: execution of make failed. Make command was: "/usr/bin/gmake" "cmTC_b8db5/fast"
-- Performing Test CXX_HAS_WARNING_-Wno-error=delete-non-virtual-dtor - Failed
-- Performing Test CXX_HAS_WARNING_-Wno-error=sign-compare
CMake Error: Generator: execution of make failed. Make command was: "/usr/bin/gmake" "cmTC_edb29/fast"
-- Performing Test CXX_HAS_WARNING_-Wno-error=sign-compare - Failed
-- Performing Test CXX_HAS_WARNING_-Wno-error=missing-declarations
CMake Error: Generator: execution of make failed. Make command was: "/usr/bin/gmake" "cmTC_82851/fast"
-- Performing Test CXX_HAS_WARNING_-Wno-error=missing-declarations - Failed
-- UPDATE TRANSLATIONS: OFF
-- Found Qt 5.6.1
-- Found Protobuf: /usr/lib64/libprotobuf.so  
-- Mysql connector NOT FOUND: servatrice won't be able to connect to a mysql server
-- Configuring incomplete, errors occurred!
See also "/builds/cockatrice/cockatrice/build/CMakeFiles/CMakeOutput.log".
See also "/builds/cockatrice/cockatrice/build/CMakeFiles/CMakeError.log".

(the mysql line is another thing I'd like to fix before merging) actually, this shows up in the official builds, so i won't worry about it for now

@ctrlaltca
Copy link
Contributor

See also "/builds/cockatrice/cockatrice/build/CMakeFiles/CMakeOutput.log".
See also "/builds/cockatrice/cockatrice/build/CMakeFiles/CMakeError.log".

Probably these files contains the exact error

@skwerlman
Copy link
Contributor Author

I would assume so, but getting at those files is an issue, since it's only happening in CI.

@skwerlman
Copy link
Contributor Author

after fixing a couple of the other issues from above, the cmake error went away.
assuming it doesn't magically crop back up, i'll update this pr and it should be good to go

fix artifact selection
fix rpm builds
mostly fix debian builds (still have cmake issue)
update to conform with Cockatrice#2373
combined rc and debug stages (more efficient as one stage)
add unused deploy section (can be finished/configured later)
@skwerlman
Copy link
Contributor Author

skwerlman commented Mar 20, 2017

I've updated the file with the requests/fixes. Please re-review.

The only current issues are that the Ubuntu 1610 and Debian Stretch debug builds fail because of their CMake version (see #2343)

An example pipeline resulting from this config can be seen here

.gitlab-ci.yml Outdated
- mkdir build || true # allowed to fail
- cd build
- cmake .. -DWITH_SERVER=1 -DCMAKE_BUILD_TYPE=Release -DCPACK_GENERATOR=DEB
- make package -j2
Copy link
Member

Choose a reason for hiding this comment

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

Use -j instead of -j2 to match threads to the number of cores instead of always 2

Copy link
Member

Choose a reason for hiding this comment

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

Can't you mkdir -p build instead of the || true aspect?

Copy link
Member

Choose a reason for hiding this comment

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

will -j automatically make use of all cores @Daenyth ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor Author

@skwerlman skwerlman Mar 21, 2017

Choose a reason for hiding this comment

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

I use -j2 here because Gitlab Runner will starve the rest of the server if I don't limit the number of jobs.

Copy link
Member

Choose a reason for hiding this comment

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

@Daenyth so is this statement on the top of our compiling wiki still correct and useful then?
https://github.com/Cockatrice/Cockatrice/wiki/Compiling-Cockatrice-(Linux)

Copy link
Member

Choose a reason for hiding this comment

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

It's not so useful. Want to remove it and change all the js?

Copy link
Member

Choose a reason for hiding this comment

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

It's not so useful. Want to remove it and change all the js?

did so for mac and linux compiling wiki


Comments from dae and zach on this section still stand.^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mkdir -p build

Oh, duh. Dunno why I did it the other way.

- apt-get -o dir::cache::archives="cache" install -y qt5-default qttools5-dev qttools5-dev-tools
- apt-get -o dir::cache::archives="cache" install -y qtmultimedia5-dev libqt5multimedia5-plugins
- apt-get -o dir::cache::archives="cache" install -y libqt5svg5-dev libqt5sql5-mysql
- apt-get -o dir::cache::archives="cache" install -y libqt5websockets5-dev
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to do multiple commands here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly so it looks nice in the yml. Splitting it up doesn't have any real impact on performance.

Copy link
Member

Choose a reason for hiding this comment

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

ok

.gitlab-ci.yml Outdated
<<: *install_requirements_16xx
<<: *artifacts_deb
<<: *cache
# <<: *bintray_releases_deb
Copy link
Member

Choose a reason for hiding this comment

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

🔪 this isn't coming back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch

.gitlab-ci.yml Outdated

#=================================== DEPLOY ===================================

# deploy_to_bintray: # currently unused; requires some config
Copy link
Member

Choose a reason for hiding this comment

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

🔪

simplify `mkdir` command
remove bintray deploy
@woogerboy21
Copy link
Contributor

I'm still a fan of this, though it looks like the windows builds failed due to the issue we had in our appveyor config. If you merge in master it should build ok.

Copy link
Contributor

@ctrlaltca ctrlaltca left a comment

Choose a reason for hiding this comment

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

Code looks ok, release builds works fine and seems to produce working artifacts.
Ubuntu 16.10 and Debian Stretch debug builds are broken because of #2343, but that can only be fixed by the ubuntu/debian maintainers by updating the packages in their distro to actually working versions.
Ubuntu bug report: https://bugs.launchpad.net/ubuntu/+source/protobuf/+bug/1643959
Debian bug report: currently not reported, nobody sane wants to mess with their archaic bugreport tool.

@ZeldaZach
Copy link
Member

Should we :shipit: on this? Would we be hosting the gitlab ourselves? Also, will they auto-upload to the release cycle with tags (cite: #2550 )

@skwerlman
Copy link
Contributor Author

I can add the code to upload to github on tags and hand over the repo on my instance to one of you guys, if you'd like it to do so. Otherwise, the builds produced by my instance would have to be manually uploaded to github.

@ctrlaltca
Copy link
Contributor

I'd merge this as it is by now; i suppose user or developer-provided builds will be considered "unofficial" by now as per #2345 (comment)
This ca be reconsidered later

@ctrlaltca ctrlaltca merged commit 6d07709 into Cockatrice:master Mar 31, 2017
@skwerlman
Copy link
Contributor Author

Where should my developer build be linked? Do we want to link them from a wiki page with the appropriate warnings about how these builds are unsupported?

@Daenyth
Copy link
Member

Daenyth commented Mar 31, 2017

For now let's go with that

@skwerlman
Copy link
Contributor Author

Here's the wiki page: https://github.com/Cockatrice/Cockatrice/wiki/Unofficial-Developer-Builds

The download links automatically link to the newest artifacts built from master.

tooomm added a commit that referenced this pull request Jun 29, 2020
@tooomm tooomm mentioned this pull request Jun 29, 2020
ebbit1q pushed a commit that referenced this pull request Jul 1, 2020
* revert #2345

* remove gitlab yml
longagofaraway pushed a commit to longagofaraway/Cockatrice that referenced this pull request Aug 14, 2020
* move db udpate (Cockatrice#4015)

* Remove gitlab config (Cockatrice#4037)

* revert Cockatrice#2345

* remove gitlab yml

* fix message when moving cards to bottom of library (Cockatrice#4006)

* Change method of opening directories to be the same for all oses, including linux (Cockatrice#4046)

* add opening directory in file browser to linux

this uses QDesktopServices to open the url "file://[location]"
by default this is
"file://$HOME/.local/share/Cockatrice/Cockatrice/pics/CUSTOM"

any distro that has a file browser should have an accompanying mime type
specifying the file handler for the file:// protocol using the
inode/directory mime type

see https://specifications.freedesktop.org/shared-mime-info-spec/shared-mime-info-spec-latest.html

if a user were to have removed their mime database this will not work and
it will fail with nothing but a log message, this would be rare and not
worth checking in my opinion

* make opening directories the same for all oses

* sort headers

* Made user information window resizable (Cockatrice#4009)

* Added horizontal layout and stretch for player icon (Cockatrice#4052)

* Fix unresolved symbols when link tests to system libgtest-dev (Cockatrice#4055)

* Enable parallel compilation. (Cockatrice#4057)

* travis: update macos 10.15 images (Cockatrice#4059)

* Fix release tests (Cockatrice#4063)

Co-authored-by: tooomm <tooomm@users.noreply.github.com>
Co-authored-by: Lee Tran <54418451+LeeTranMN@users.noreply.github.com>
Co-authored-by: ebbit1q <ebbit1q@gmail.com>
Co-authored-by: awlangham <awlangham@gmail.com>
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

6 participants