Closed Bug 364628 Opened 18 years ago Closed 18 years ago

AUS should allow regular expression matches for nightly version mappings

Categories

(AUS Graveyard :: Administration, task)

task
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
4.x (triaged)

People

(Reporter: morgamic, Assigned: dao)

References

Details

Attachments

(1 file)

In bug 364477 comment #3 a point is made for simplifying the version config so that when upgrades are made along a branch version the mappings will remain consistent without requiring a manual edit. Currently, our process is very manual, meaning that if we currently have: $branchVersions = array( '3.0a1' => 'foo' ); ...and we want to upgrade that to 3.0a2, it would require a patch for the AUS config: $branchVersions = array( '3.0a1' => 'foo', '3.0a2' => 'foo' ); Ideally, we could just set something like '3.0.*' => 'foo' and key off of the main version. In order to do this, the following would have to happen: * modify getBranch() in patch.class.php to return the correct branch using a regular expression match on the members in $branchVersions compared to the incoming client version. * determine how to handle multiple matches (this is a possible bug that could be introduced if any expression was too greedy) The goal here would be to avoid discontinuity in nightly updates due to a stupid config change that probably doesn't need to happen every time a version is bumped.
A related issue is that Sunbird is now using nightly updates, so at some point it may start to collide with the mappings used for Firefox & Thunderbird. It's not a problem at the moment, because Sunbird is at v0.4a1 and heading towards a 0.5 release, while the Fx/Tb entries up to 1.5 are so old they can be safely be wiped. But maybe we should be thinking about this now and putting some per-app mapping functionality in at the same time.
I think getBranch() could look like this: > function getBranch($version) { > foreach ($this->branchVersions as $versionPattern => $branch) { > // No need to create a regular expression and test against it if there's no wildcard > if (strpos ($versionPattern, '*') === false) { > if ($versionPattern == $version) { > return $branch; > } > } elseif (preg_match('/^'. str_replace('\\*', '.*', preg_quote($versionPattern, '/')) .'$/', $version)) { > return $branch; > } > } > return false; > } Then $branchVersions could be: array( '0.4a1' => 'trunk', '0.5' => 'trunk', '1.0+' => '1.5', '1.4*' => '1.5', '1.5' => '1.5', '1.5.0.*' => '1.5.0.x', '1.6a1' => 'trunk', '2.0*' => '2.0', '3.0a*' => 'trunk' ) or even: array( '0.4a1' => 'trunk', '0.5' => 'trunk', '1.5.0.*' => '1.5.0.x', '1.6a1' => 'trunk', '1.*' => '1.5', '2.0*' => '2.0', '3.0a*' => 'trunk' ) (In reply to comment #0) > * determine how to handle multiple matches (this is a possible bug that could > be introduced if any expression was too greedy) IMHO returning the first match is fine. The patterns have to be valid and well considered, because they are part of the algorithm. If they are flawed, you're lost anyway. How could the function decide what's right?
Attached patch patch (deleted) — Splinter Review
this also addresses comment 1. mind if I work on / take this bug?
Attachment #249428 - Flags: first-review?(morgamic)
Go for it -- patch looks good, and I can review it and run the acceptance tests in the morning. :) Thanks!
Assignee: morgamic → bugzilla
Status: NEW → ASSIGNED
Comment on attachment 249428 [details] [diff] [review] patch The patch broke two nightly acceptance test cases, I'm looking into why that happened. It looked good otherwise.
Attachment #249428 - Flags: first-review?(morgamic) → first-review-
Comment on attachment 249428 [details] [diff] [review] patch The two tests that failed were the nightly update tests, and that was because the test config did not contain a "Synthetic" product for use with the nightly test snippets (they use made-up products/versions for the tests). I added a few more products and versions, and things worked great. Thanks for writing the patch.
Attachment #249428 - Flags: first-review- → first-review+
I've checked this in but have not tagged it for STAGING or PRODUCTION yet. I'd like preed or rhelmer to take a look next week.
Blocks: 364589
It would be good to get this into production, combined with finishing changing the Sunbird version on the trunk (bug 365342), because there are Sunbird Linux builds from the 1.8 branch and trunk mingling in the update system. More details in bug 364589 comment #7.
Blocks: 367380
morgamic: Can we get this pushed to production? It's blocking a couple important bugs now.
Yessir, we can get this pushed first thing Monday. I've already seen the acceptance tests, and they look fine. Would like to get preed or rhelmer to take a look first. I'll tag it for STAGING.
Tagged and ready to go. Talked to preed and rhelmer about it as well.
Depends on: 367804
This was deployed today.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
No longer blocks: 367380
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: