Opened 2 years ago

Last modified 10 months ago

#5133 new change

Add CI for libadblockplus

Reported by: hfiguiere Assignee:
Priority: Unknown Milestone:
Module: Libadblockplus Keywords:
Cc: fhd, sergz, oleksandr Blocked By: #5433, #7068
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29498563/

Description (last modified by sergz)

Background

It would help a lot if we could run CI for libadblockplus. We can use AppVeyor and TravisCI and get it covered that way, since we have automatic mirroring from our Mercurial to Github.

What to change

Grab Sergei's patch to add the proper config files for AppVeyor and Travis and make them part of master. The latest state of the patch is the last commit of ci-master branch in my fork of libadblockplus, https://github.com/abby-sergz/libadblockplus/tree/ci-master.

Change History (18)

comment:1 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:2 Changed 2 years ago by sergz

  • Description modified (diff)

If we do it this way I have a couple of questions:

  • should we add labels into readme file?

I would be glad to extend https://github.com/kzar/eyeo-helpers with functionality augmenting codereview with results from CI-servers (I will likely do it regardless of this issue).

  • I would like to have an option (actually access rights) to force rebuild (merely to run the tests again) of the public repo because on practice so far tests are failing very often. (we will be able to really fix them if it's still needed after finishing with detached threads #3595 and Co.). However it still makes sense to force rebuild because of occasional issues with CI-servers (e.g. travis is experiencing serious technical issues at least once each month).
  • what about uploading of binaries, in particular it concerns libadblockplus-binaries? I have some thoughts about it and I think it would be better to discuss it separately, here I just want to mention that I think it's fine to have even a hacky solution for it and I think it makes sense to trigger it manually, not each time when master in libadblockplus changes a commit it points to.
  • and what is actually the status of our internal CI system?

comment:3 Changed 2 years ago by fhd

We already use Circle CI in another project, so I would suggest to use that here rather than Travis.

and what is actually the status of our internal CI system?

I'll look into whether it makes sense to wait for that or not. My hunch is: No. But I can say more in a few hours.

comment:4 follow-up: Changed 2 years ago by fhd

Took more than a few hours but yes, let's for now use some cloud CI server. We can still switch to something internal later once we have it, but that's not really around the corner.

As for Circle vs Travis vs AppVeyor, can we move the discussion about the pros and cons here? We don't _have_ to use Circle as I said, I'd just say that when in doubt, it makes sense to use something we already use for something else rather than something entirely new.

comment:5 in reply to: ↑ 4 Changed 2 years ago by sergz

Replying to fhd:

As for Circle vs Travis vs AppVeyor, can we move the discussion about the pros and cons here? We don't _have_ to use Circle as I said, I'd just say that when in doubt, it makes sense to use something we already use for something else rather than something entirely new.

Firstly about AppVeyor, it seems the only one option for windows builds and the working config is already here.

Circle vs Travis:
TL;DR: With the present information I'm for travis.

For reference (from IRC): I don't mind about Circle, which has a pretty interesting feature set which can be potentially useful for us especially in a long term, though I doubt that we cannot achieve the same goals with travis. My main concern, which is based on negative experience with single-build AppVeyor, is only the waiting time. We could use a paid account on Circle but it seems strange when anyone can get the same result for free.

What concerns about something entirely new, I find it subjective, e.g. I use travis for libadblockplus for longer than one year (at least the commit I'm modifying is created longer than a year ago, but I had started even before), secondly, I think that most people have never even heard about circle ci but are already used to travis, so rather circle ci seems something entirely new. Maybe it even makes sense to switch another project from circle to travis.

comment:6 Changed 2 years ago by fhd

Firstly about AppVeyor, it seems the only one option for windows builds and the working config is already here.

Windows support is nice, if we don't have that, build/test issues will only be noticed when we compile ABP for IE. Not super important, but definitely nice to have.

So, given how we already have the AppVeyor config - let's go for that? Let me know via email whether I need to create accounts or something.

What concerns about something entirely new, I find it subjective, e.g. I use travis for libadblockplus for longer than one year (at least the commit I'm modifying is created longer than a year ago, but I had started even before), secondly, I think that most people have never even heard about circle ci but are already used to travis, so rather circle ci seems something entirely new. Maybe it even makes sense to switch another project from circle to travis.

We don't use it for ABP, that's what I meant - private usage is a different matter.

comment:7 follow-up: Changed 2 years ago by fhd

I suppose if we're looking into Windows builds, it could make sense to see if we can tackle #1092 while at it.

comment:8 Changed 2 years ago by fhd

  • Blocking 1092 added

comment:9 in reply to: ↑ 7 Changed 2 years ago by sergz

  • Cc oleksandr added

Replying to fhd:

I suppose if we're looking into Windows builds, it could make sense to see if we can tackle #1092 while at it.

The issue with third-party build servers is in a security aspect. Until we don't sign binaries or sign them with an unofficial certificate it's not an issue (in particular I guess it's pretty trivial now to create a config for appveyor for ABP-for-IE, but I guess Oleksandr has something to add). JIC, there is a nuance, which I find important, which is we need to have signed binaries in the middle of the build process, because we need them before we build msi package which can be signed for instance later somehow.

comment:10 Changed 2 years ago by fhd

Oh, I thought that we can produce non-signed binaries and then sign these in a separate step. We could still give AppVeyor credentials to our signing server, but the more important question is whether we want to.

I guess let's tackle this first and worry about #1092 later then.

comment:11 Changed 2 years ago by fhd

  • Blocking 1092 removed

comment:12 Changed 2 years ago by sergz

  • Blocked By 5433 added

comment:13 Changed 2 years ago by sergz

Since it takes minutes to run CI with prebuilt V8 I think for present we can support both travis-ci and circle-ci (more CI is never worse) and simply abandon one if it ever does not fit our requirements.

comment:14 Changed 2 years ago by sergz

  • Review URL(s) modified (diff)

comment:15 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5133 - add CI for libadblockplus

comment:16 Changed 22 months ago by oleksandr

Looks like this can be closed now?

comment:17 Changed 22 months ago by sergz

It's not closed in order to be a place for what else should be done and to give the historical overview. Feel free to transform it to a meta issue and create corresponding specific issues.

In particular it would be good to

  • have an icon in readme on github with the latest status
  • resolve the issue with appveyor account (see the corresponding hub ticket)
  • perhaps add circle-ci configuration/intergration
  • implement generation of artifacts
  • align with the upcoming changes (several people told me about gitlab) or integrate it with current codereview.

comment:18 Changed 10 months ago by sergz

  • Blocked By 7068 added
Note: See TracTickets for help on using tickets.