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)
Tracking
(Not tracked)
RESOLVED
FIXED
4.x (triaged)
People
(Reporter: morgamic, Assigned: dao)
References
Details
Attachments
(1 file)
(deleted),
patch
|
morgamic
:
first-review+
|
Details | Diff | Splinter Review |
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.
Comment 1•18 years ago
|
||
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.
Assignee | ||
Comment 2•18 years ago
|
||
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?
Assignee | ||
Comment 3•18 years ago
|
||
this also addresses comment 1.
mind if I work on / take this bug?
Attachment #249428 -
Flags: first-review?(morgamic)
Reporter | ||
Comment 4•18 years ago
|
||
Go for it -- patch looks good, and I can review it and run the acceptance tests in the morning. :) Thanks!
Reporter | ||
Updated•18 years ago
|
Assignee: morgamic → bugzilla
Assignee | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 5•18 years ago
|
||
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-
Reporter | ||
Comment 6•18 years ago
|
||
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+
Reporter | ||
Comment 7•18 years ago
|
||
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.
Comment 8•18 years ago
|
||
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.
Comment 9•18 years ago
|
||
morgamic:
Can we get this pushed to production? It's blocking a couple important bugs now.
Reporter | ||
Comment 10•18 years ago
|
||
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.
Reporter | ||
Comment 11•18 years ago
|
||
Tagged and ready to go. Talked to preed and rhelmer about it as well.
Reporter | ||
Comment 12•18 years ago
|
||
This was deployed today.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•