Closed Bug 940402 Opened 11 years ago Closed 10 years ago

Calculate Lightning version from Thunderbird version

Categories

(Calendar :: Build Config, defect)

Lightning 2.6
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Fallen, Assigned: Fallen)

Details

Attachments

(1 file, 1 obsolete file)

There have been multiple 2.6.x releases that overwrite and old release, because the version was not bumped. While this is an oversight that could be taken care of, I'd rather do the same thing we do in some other places: calculate the version number instead. 

This might mean we need to adapt our versioning schema a bit, the following cases need to be covered:

31.0   -> 3.3
31.0.1 -> 3.3.0.1
31.1.0 -> 3.3.1
31.1.1 -> 3.3.1.1


This already won't work for 2.6.x, because Thunderbird 24.1.1 needs Lightning 2.6.3, so I would suggest to use this schema for comm-esr24 only:

24.0   -> 2.6
24.0.1 -> 2.6.1
24.1.0 -> 2.6.2
24.1.1 -> 2.6.3
<automation begins here>
24.2.0 -> 2.6.4
24.2.1 -> 2.6.4.1
24.3.0 -> 2.6.5

Stefan, what do you think?
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
So this patch should take care. Asking Callek for review because it touches a file in suite/ and mail/. I'm also fine with his review for the remaining parts.

Callek, can you review all parts, or do I need someone else for the mail/ changes?
Attachment #8337214 - Flags: review?(bugspam.Callek)
Attachment #8337214 - Flags: feedback?(ssitter)
Comment on attachment 8337214 [details] [diff] [review]
Fix - v1

Review of attachment 8337214 [details] [diff] [review]:
-----------------------------------------------------------------

::: calendar/providers/gdata/Makefile.in
@@ +18,1 @@
>  CALENDAR_VERSION := $(shell cat $(topsrcdir)/calendar/sunbird/config/version.txt)

This line should be removed because it is added in a new version three lines below.
Ah thanks, looks like I missed that one!
What about Beta builds? It seems that Thunderbird uses version e.g. 26.0 for all of its Beta builds and the release builds, i.e. all Lightning builds would be labeled as 2.8 instead of 2.8b1, 2.8b2, ...
Comment on attachment 8337214 [details] [diff] [review]
Fix - v1

Oooh right...Ok, I will figure out how to get the full version with the beta number.
Attachment #8337214 - Attachment is obsolete: true
Attachment #8337214 - Flags: review?(bugspam.Callek)
Attachment #8337214 - Flags: feedback?(ssitter)
Attached patch Fix - v2 β€” β€” Splinter Review
Ok, this one should do it. The beta builds set MOZ_PKG_VERSION=24.0b3 for example. The patch prefers MOZ_PKG_VERSION over THUNDERBIRD_VERSION to make sure the betas get the right version number.
Attachment #8337277 - Flags: review?(bugspam.Callek)
Attachment #8337277 - Flags: feedback?(ssitter)
Comment on attachment 8337277 [details] [diff] [review]
Fix - v2

I won't have time this week to review this, but if Joshua can beat me to the review you can use his over my own.
Attachment #8337277 - Flags: review?(Pidgeot18)
Comment on attachment 8337277 [details] [diff] [review]
Fix - v2

Review of attachment 8337277 [details] [diff] [review]:
-----------------------------------------------------------------

::: calendar/lightning/build/makeversion.py
@@ +10,5 @@
> +MINOR_ADD=0
> +
> +def makeversion(x):
> +  parts = x.split('.')
> +  parts[0] = str((float(parts[0]) + 2) / 10)

I'm extremely wary of using float here, given its happiness for generating things like 2.300000000004. Given that the equivalent logic can easily be written with integers and string manipulation, I'd rather you use that instead.

@@ +13,5 @@
> +  parts = x.split('.')
> +  parts[0] = str((float(parts[0]) + 2) / 10)
> +  if len(parts) > 1 and parts[1].isdigit():
> +    parts[1] = str(int(parts[1]) + MINOR_ADD)
> +  return re.sub(r'.0([ab][0-9]*|)$', r'\1', '.'.join(parts))

So this would make 24.2.1 become 2.6.2.1... is that you want?

@@ +15,5 @@
> +  if len(parts) > 1 and parts[1].isdigit():
> +    parts[1] = str(int(parts[1]) + MINOR_ADD)
> +  return re.sub(r'.0([ab][0-9]*|)$', r'\1', '.'.join(parts))
> +
> +print makeversion(sys.argv[1])

Please use print(makeversion(sys.argv[1])) instead. I know gps is trying to push for python3-ish compatibility, and this is one of those simple things that needs to be done to avoid this.
Attachment #8337277 - Flags: review?(Pidgeot18)
Comment on attachment 8337277 [details] [diff] [review]
Fix - v2

(In reply to Joshua Cranmer [:jcranmer] from comment #8)

> 
> @@ +13,5 @@
> > +  parts = x.split('.')
> > +  parts[0] = str((float(parts[0]) + 2) / 10)
> > +  if len(parts) > 1 and parts[1].isdigit():
> > +    parts[1] = str(int(parts[1]) + MINOR_ADD)
> > +  return re.sub(r'.0([ab][0-9]*|)$', r'\1', '.'.join(parts))
> 
> So this would make 24.2.1 become 2.6.2.1... is that you want?
Yes, anything else will make tracking versions hard because its unclear how many b there are in 24.a.b


I've discussed with jcranmer that there might be a few more followup bugs depending on what direction the mozbuild changes go, but this patch is r+ with the changes he mentioned.
Attachment #8337277 - Flags: review?(bugspam.Callek) → review+
Pushed to comm-central changeset 8d82423a804f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.1
I'll push this to the other branches once I see this work on trunk.
Attachment #8337277 - Flags: feedback?(ssitter)
You need to log in before you can comment on or make changes to this bug.