Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add Linux CI On Travis #22

Merged
merged 4 commits into from
Nov 20, 2015
Merged

Add Linux CI On Travis #22

merged 4 commits into from
Nov 20, 2015

Conversation

joefitzgerald
Copy link
Contributor

This PR:

  • Updates .travis.yml to include a build matrix for linux and osx
  • Updates .travis.yml to include a build matrix for stable and beta channels
  • Updates build-package.sh to work when the OS is linux
  • Updates build-package.sh to work with a variable number of channels (i.e. if additional channels are added (e.g. alpha) then it should Just Work™
  • Fixes Add Linux CI support #2

In the future, further enhancement could be possible on Linux if there were a .deb repository for atom's .deb releases:

addons:
  apt:
    packages:
    - atom

/cc @nathansobo @kevinsawicki @thedaniel

@Arcanemagus
Copy link
Contributor

By the way, you can remove the language: objective-c line since you are specifying the os now. (It saves a tiny step during preparation, but otherwise makes no difference if it is removed.)

@nathansobo
Copy link

@joefitzgerald Can you link to an example of these scripts working on a package? Are you using these on go-plus now?

@maxbrunsfeld
Copy link
Contributor

@joefitzgerald ❤️ This is great. Getting things working on linux is huge.

I think that for most packages, building on both linux and osx might add more slowness than it's worth, since package code usually doesn't depend strongly on the OS. What do you think about just commenting out the osx line in the os matrix so that by default, packages will just build on linux? For packages where the OS is a major concern, the author will just have to uncomment that line.

@Arcanemagus
Copy link
Contributor

OS X is actually where I've seen the most issues (Mainly due to it's silly path handling), and as Travis is the easiest place to get it testing on I really think it should be left on there.

Besides, the builds run in parallel so it doesn't add that much total time to run both 😉

@maxbrunsfeld
Copy link
Contributor

Besides, the builds run in parallel so it doesn't add that much total time to run both

I think it does add significant time, because w/ 4 builds running for every commit instead of 1, builds are likely to remain queued for a long time before they start. Am I missing something?

Regarding OSX's case-insensitive paths, that's an interesting point. I haven't come across many issues due to that, but I can see how it would be an issue for certain packages.

@Arcanemagus
Copy link
Contributor

Actually I was referring to how if launched from the GUI it has no environment (thus utilities can't be found), while if you launch if from a terminal it properly inherits that environment. In the linter-* packages that I mainly deal with we have a lot of issues reported because of it.

I've only been testing with 2 builds, 4 probably does take longer to queue.

@maxbrunsfeld
Copy link
Contributor

if launched from the GUI it has no environment

I see. That makes more sense. Doesn't travis launch it from the terminal anyway though, regardless of platform?

@Arcanemagus
Copy link
Contributor

Yea, that aspect isn't really tested here, I've only seen one issue that was OS X specific that was exposed on Travis.

@joefitzgerald
Copy link
Contributor Author

@maxbrunsfeld it's important to note that the package author:

  1. Has to copy the .travis.yml to their own repo (and consequently is free to comment out / remove whatever they want)
  2. Have their own Travis accounts (they aren't using Atom's)
  3. Usually have a low volume of changes in their account, so build concurrency is not that big an issue
  4. Have specs that usually run very fast

I think it would be better to err on the side of testing more, and let package authors opt out if they run into issues with builds backing up. Look here for an example of elapsed duration for a typical package (which does relatively complex things in the install phase related to go):

We're talking about < 2 minute build times.

joefitzgerald added a commit to joefitzgerald/go-config that referenced this pull request Nov 20, 2015
@joefitzgerald
Copy link
Contributor Author

@Arcanemagus Take a look at https://atom.io/packages/environment. It provides a service that - if consumed - will be sane, even if launched from Dock, Finder, Spotlight, or open.

@joefitzgerald
Copy link
Contributor Author

@nathansobo: @joefitzgerald Can you link to an example of these scripts working on a package? Are you using these on go-plus now?

https://travis-ci.org/joefitzgerald/go-config/builds/92339238

export CI_APM_SCRIPTNAME="apm-$ATOM_CHANNEL"
fi
export CI_ATOM_SH="/usr/bin/$CI_ATOM_SCRIPTNAME"
export CI_APM_SH="/usr/bin/$CI_APM_SCRIPTNAME"
Copy link
Contributor

Choose a reason for hiding this comment

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

What does SH stand for in these variable names? What do you think of these names:

ATOM_SCRIPT_NAME
ATOM_SCRIPT_PATH
APM_SCRIPT_NAME
APM_SCRIPT_PATH

Regarding the CI prefix, I'd probably leave it off unless you know of a specific case where it would collide with some other known ATOM_ environment variable.

sudo apt-get install xdg-utils -qq
sudo apt-get install libcap2 -qq
sudo apt-get install gdebi-core -qq
sudo apt-get update -qq
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you call apt-get update before and after installing all the packages. Do you need to apt-get update again afterwards?

@maxbrunsfeld
Copy link
Contributor

Look here for an example of elapsed duration for a typical package (which does relatively complex things in the install phase related to go.

👍 That's awesome to see. OK, I see your point about package authors generally having plenty of queue space.

I left a bunch of pretty minor feedback, but other than that, I'm down to merge this asap. 📈!

@joefitzgerald
Copy link
Contributor Author

Sooo, I figured out how to run in containerized infrastructure! Linux builds now ~50% shorter: https://travis-ci.org/joefitzgerald/go-config/builds/92351552 🎉

Also, addressed your suggestions @maxbrunsfeld.

@maxbrunsfeld
Copy link
Contributor

😭 🌈 Tears of joy. Brilliant.

curl -s -L "https://atom.io/download/mac?channel=$ATOM_CHANNEL" \
-H 'Accept: application/octet-stream' \
-o "atom.zip"
ATOM_DOWNLOAD_URL="https://atom.io/download/mac?channel=$ATOM_CHANNEL"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can 🔥 this variable now.

@joefitzgerald
Copy link
Contributor Author

:shipit:

maxbrunsfeld pushed a commit that referenced this pull request Nov 20, 2015
@maxbrunsfeld maxbrunsfeld merged commit e8fa63c into master Nov 20, 2015
@maxbrunsfeld
Copy link
Contributor

Thanks @joefitzgerald!

@maxbrunsfeld maxbrunsfeld deleted the jf-add-linux-ci branch November 20, 2015 23:17
@50Wliu
Copy link
Contributor

50Wliu commented Nov 20, 2015

🐧 💚 💯

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants