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

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

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

mgol
Copy link
Member

@mgol mgol commented Apr 24, 2019

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
  • Reviewing ua-parser/uap-php@v3.4.7...v3.8.8
  • Basic local testing of this new ua-parser version.
  • Add composer.lock file on master.
  • Pull down from master on swarm.jquery.org for lockfile (no-op).
  • Figure out whether Composer is still installed, a decent and secure version of it, and that it is all compatible with PHP 5.4, then use Composer on the server to re-create the --no-dev vendor directory from the new lockfile.
  • Merge this PR (rebased with updated lockfile).
  • ~~Pull down on swarm.jquery.org and use Composer to no-dev install updated vendor from lock file. ~~

@Krinkle Krinkle self-assigned this Apr 24, 2019
@Krinkle
Copy link
Member

Krinkle commented Apr 24, 2019

Reviewed the upgrade.

  • Code looks fine and secure still. Changes limited to internal methods that shouldn't affect our calls to the library.
  • Use of PHP 5.4 (which we use on this ancient server) is still supported, integrated via their Travis CI, and passing.
  • A new dependency was added, composer/ca-bundle. No other (indirect) dependencies exist or were changed, and the new dependency also has no further dependencies.
  • composer/ca-bundle: The file ca-bundle.crt was previously shipped directly and (some other version of) that file is now brought in from the composer org instead of maintained by ua-parser. The main difference is that it no longer uses the Mozilla CA bundle unconditionally for fetching updates. Instead, it will now first try to find a CA bundle on the system. This might end up being a problem, since we don't regularly update it on this server afaik and might be unable to establish connections in that case. We'll see. Afaik we don't run the updater regularly anyway, so might be ignorable.

@mgol
Copy link
Member Author

mgol commented Apr 24, 2019

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

@Krinkle
Copy link
Member

Krinkle commented Apr 24, 2019

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

mgol commented Jun 27, 2019

@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
Copy link
Member

Krinkle commented Jun 27, 2019

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

mgol commented Jun 27, 2019 via email

@Krinkle
Copy link
Member

Krinkle commented Jun 28, 2019

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

mgol commented Jul 1, 2019

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

@Krinkle
Copy link
Member

Krinkle commented Jul 14, 2019

@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
Copy link
Member Author

mgol commented Nov 2, 2019

@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
Copy link
Member Author

mgol commented Dec 18, 2019

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

@Krinkle
Copy link
Member

Krinkle commented Dec 18, 2019

@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 added a commit that referenced this pull request Dec 18, 2019
* Remove polyfills of php extensions we assume are installed
  (and indeed are installed in production).
* Regenerate lock file and audit dependency changes.
* Commit dependencies for ease of deployment and review.

Ref #319.
@Krinkle
Copy link
Member

Krinkle commented Dec 18, 2019

Now deployed.

krinkle@/var/www/swarm$ sudo mv vendor/ ../swarm-vendor-backup && sudo git pull

@mgol
Copy link
Member Author

mgol commented Dec 19, 2019

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

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
@mgol mgol changed the title build: Update ua-parser build: Update ua-parser to recognize the Chromium-based Edge & Android 9 Dec 19, 2019
@Krinkle Krinkle merged commit a20c993 into jquery:master Dec 19, 2019
@mgol mgol deleted the ua-parser branch December 19, 2019 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants