jquery / testswarm

Distributed continuous integration testing for JavaScript.
http://swarm.jquery.org
Other
967 stars 158 forks source link

build: Update ua-parser to recognize the Chromium-based Edge & Android 9 #319

Closed mgol closed 4 years ago

mgol commented 5 years ago

This is needed to properly recognize:

  1. the Chromium-based Edge which uses the "Edg/" token instead of "Edge/"
  2. Android 9 which doesn't use a dot in its version in the user agent string
Krinkle commented 5 years ago

Reviewed the upgrade.

mgol commented 5 years ago

@Krinkle I don't have write access to this repository. Can you merge it if it looks fine for you?

Krinkle commented 5 years ago

Yeah, once I get to that point I will merge it! (I don't have admin rights and thus also unable to grant it to you.)

mgol commented 5 years ago

@Krinkle Can I update uap-php to 3.9.0? The v3.8.8...v3.9.0 diff is mostly regexes changes and some are improvements for Edge parsing so I'd like to get it in.

Krinkle commented 5 years ago

That's fine sure. We can handle that at the same time.

Note that this is still blocked on several other steps (per pull description). From a quick glance, it looks like upstream no longer has a working build on PHP 5.4. Which is fair for them (it's super EOL), but also what our server still runs.

mgol commented 5 years ago

Is it enough for us that 3.8.8 had PHP 5.4 support and the code changes between 3.8.8 and 3.9.0 are so minimal? Or do we need to update PHP first? -- Michał Gołębiowski-Owczarek

Krinkle commented 5 years ago

The v3.8.8...v3.9.0 changes are minimal indeed and do not pose any concern. The v3.4.7...v3.8.8 diff is less trivial, and I'll need to re-review those in light of PHP 5.4 compat. But, we could just try, and revert if we find issues. We'll need to ensure Composer still works, which I can do once time is on my side :)

Regarding PHP and Composer, let's continue in private at https://github.com/jquery/infrastructure/issues/444.

mgol commented 5 years ago

@Krinkle PR updated. I had to run composer update & lots of things got removed from the lockfile, is that normal?

Krinkle commented 5 years ago

@mgol It's normal to the extent that it reflects the change that ua-parser has undergone. Upstream removed several of its (allegedly unused) dependencies.

mgol commented 4 years ago

@Krinkle It looks like the Chromium Edge may near a stable release so I had another look at it (as it's not clear when https://github.com/jquery/infrastructure/issues/449 gets resolved):

  1. Composer is tested on PHP 5.3+ even on master: https://github.com/composer/composer/blob/master/.travis.yml. The latest version should work with our PHP 5.4 then.
  2. We don't seem to have it installed on the swarm machine. The installation instructions involve 4 commands, see https://getcomposer.org/download/. Is it OK to run them directly on the host?
  3. As for installing the dependencies on the swarm host, the swarm directory is owned by root and running Composer by root is not advised. Should we run composer on a regular account and then just move the files?

The diff between 3.9.0 that this PR currently uses & 3.9.1 is just one script so I'd do that update as well in this PR.

mgol commented 4 years ago

@Krinkle could you weigh in? The new Edge will arrive in less than a month.

Krinkle commented 4 years ago

@mgol

  1. Looks like Composer has come back from not supporting PHP 5.3. That is interesting to see. Good news.

  2. Composer is installed, but not globally (and indeed should not be as root). I have a copy of it in my home directory, that's how I used it so far. But, moving the files is tedious indeed. Perhaps we can just check them into Git. That seems like a better plan (it's not nearly as bad as node_modules).

  3. With the composer.lock checked in and platform versions pinned (by default Composer's dependency tree can expand differently based on your OS/php version, php extensions etc.), it is fine to run locally with a different PHP than in prod. Thus making check-in quite feasible. I'll get on this now.

Krinkle commented 4 years ago

Now deployed.

krinkle@/var/www/swarm$ sudo mv vendor/ ../swarm-vendor-backup && sudo git pull
mgol commented 4 years ago

@Krinkle I rebased my PR and then:

  1. I kept my change in composer.json but bumped the ua-parser/uap-php version from 3.9.0 to 3.9.2. The diff here is mostly the regexes update, including the addition of KaiOS in 3.9.2 so it looks safe.
  2. I reverted composer.lock to the state on master & run composer update to update it again.
  3. The previous step heavily modified the vendor directory. I reset it to the master state & run composer install --no-dev to only update production dependencies. This removed some folders like psr or symfony, I hope that's fine.

What are the next steps here? Can I help with anything?