Closed Bug 385485 Opened 17 years ago Closed 17 years ago

Automate version number (and year) updates in mozilla/camino

Categories

(Camino Graveyard :: General, defect)

Product:
Camino Graveyard
Component:
General
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: alqahira, Assigned: alqahira)

References

Details

(Keywords: fixed1.8.1.8, Whiteboard: [camino-1.5.2])

Attachments

(3 files, 7 obsolete files)

Trunk patch v1.1, for checkin
21.84 KB, patch
Details | Diff | Splinter Review
Branch patch v1.1, for checkin
19.60 KB, patch
Details | Diff | Splinter Review
1_5 branch version
30.06 KB, patch
mark
: superreview+
Details | Diff | Splinter Review 
Getting rid of most (all) of the 5 places we need to update this every version change or year change is next on my build system hitlist.
Attached patch branch patch, v0.8 (obsolete) — — Splinter Review 
Here's a branch patch that does this.  So far I've tested it in my debug build (save the project changes, of course), and it works.  I wiped my static tree by accident the other day, so I'll have to rebuild and test that later.

I'm not positive the project changes are correct in the new project; I'm still hacking around blind there.  If someone could check those, that'd be helpful.

Once I test my branch static and get some feedback, I'll make a trunk patch (src/embedding/CmXULAppData.h.in gets touched there, too).

Info-Camino.plist and resources/localized/English.lproj/InfoPlist.strings will need to be CVS removed.

We've made a large number of changes recently that reduce the number of places this stuff has to be changed, which is nice, but this is still useful IMO.

I didn't do anything with IBPalette/resources/English.lproj/InfoPlist.strings, either; do we care about that (© hasn't ever been updated there)?
Comment on attachment 277325 [details] [diff] [review]
branch patch, v0.8

I've converted the spaces in the Makefile into tabs...
Attached patch branch patch, 0.9 (obsolete) — — Splinter Review 
...and added Camino.xcconfig
Attachment #277325 - Attachment is obsolete: true
Attached patch branch patch, 0.9a (obsolete) — — Splinter Review 
Er, there was a stray tab on a blank line.  Sorry for the patchspam.
Attachment #277344 - Attachment is obsolete: true
Attached patch branch patch, 0.9b (obsolete) — — Splinter Review 
:sigh:  This branch patch should have all the files this time, and have un-stuck itself from the downrev Makefile.in.  Too many files for one patch :p
Attachment #277347 - Attachment is obsolete: true
Attached patch trunk patch, v0,9 (obsolete) — — Splinter Review 
Here's the trunk equivalent.  It should have all the files and corrections of the the last half-dozen branch patches :P

Everyone should be able to test this on their trunk trees, though.  Just remove Info-Camino.plist and resources/localized/English.lproj/InfoPlist.strings first.
Comment on attachment 277350 [details] [diff] [review]
trunk patch, v0,9

Unsolicited pre-review drive-by review:

Cool.  I really like this.

+CM_COPY_YEAR_FILE = $(srcdir)/config/year.txt
+CM_COPY_YEAR := $(shell cat $(CM_COPY_YEAR_FILE))

These variable names make it sound like something's being copied.  Either get rid of COPY or write COPYRIGHT.  Also applies to %VARIABLE_SUBSTITUTION%.

+generated/English.lproj:
+	mkdir -p $@

Can we just put InfoPlist.strings into generated?  I don't see why we need generated/English.lproj.

+	sed -e "s/%FOX_APP_VERSION%/$(FOX_APP_VERSION)/" \
+		-e "s/%CM_APP_VERSION%/$(CM_APP_VERSION)/" $< > $@

Line up the -e things.  Same thing applies throughout.  The first character on each continuation line like this should be a tab, additional characters needed to make it line up properly should be spaces.

+	sed -e "s/%CM_APP_VERSION%/$(CM_APP_VERSION)/" \
+		-e "s/%CM_COPY_YEAR%/$(CM_COPY_YEAR)/" $< | iconv -f UTF-8 -t UTF-16 > $@

I like seeing iconv in there.  This is a long line now, since you've established the pattern of breaking these up with trailing backslashes, you should break this after the pipe character.  My preference would be for it to look something like:

	sed -e "s/%CM_APP_VERSION%/$(CM_APP_VERSION)/" \
	    -e "s/%CM_COPYRIGHT_YEAR%/$(CM_COPYRIGHT_YEAR)/" $< | \
	  iconv -f UTF-8 -t UTF-16 > $@
Comment on attachment 277350 [details] [diff] [review]
trunk patch, v0,9

+	<key>CFBundleVersion</key>
+	<string>%CM_APP_VERSION%</string>

Shouldn't this be more along the lines of PLATFORM_BUILD_VERSION?  It's a break from what we currently do, but I think that it's the right thing to do.
(In reply to comment #7)
> +generated/English.lproj:
> +       mkdir -p $@
> 
> Can we just put InfoPlist.strings into generated?  I don't see why we need
> generated/English.lproj.

Since it has been so long, I wanted to make sure I recorded here that mento and I talked about this in the channel, and doing this makes Xcode happy with localizable files.  The alternative would be to create an entirely new Bundle Resources copy phase to handle this manually.  mento seemed to agree that generated/English.lproj was fine.
Functionality review says this works on trunk.
Attached patch Trunk patch, v1.0 (obsolete) — — Splinter Review 
This should address mento's remaining pre-review drive-by comments.  

(Thanks to Ian for verifying I didn't give Xcode indigestion with my project hackery.)
Attachment #277350 - Attachment is obsolete: true
Attachment #278367 - Flags: superreview?(mark)
Attached patch Branch patch, v1.0 (obsolete) — — Splinter Review 
Same deal, branch version.
Attachment #277349 - Attachment is obsolete: true
Attachment #278368 - Flags: superreview?(mark)
Comment on attachment 278367 [details] [diff] [review]
Trunk patch, v1.0

+generated/English.lproj:

This should depend on generated conceptually, even though you've got "mkdir -p".
Attachment #278367 - Flags: superreview?(mark) → superreview+
Comment on attachment 278368 [details] [diff] [review]
Branch patch, v1.0

As above.
Attachment #278368 - Flags: superreview?(mark) → superreview+
For checkin; fixes mento's comment about making generated/English.lproj depend on generated.

Note that resources/localized/English.lproj/InfoPlist.strings and Info-Camino.plist should be removed upon checkin.
Attachment #278367 - Attachment is obsolete: true
Checked in on trunk and MOZILLA_1_8_BRANCH
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed → fixed1.8.1.7
Resolution: --- → FIXED
This landed on branch with LSMinimumSystemVersion set to 10.4 :(
(In reply to comment #19)
> This landed on branch with LSMinimumSystemVersion set to 10.4 :(

cvs fed me just this one file as trunk from this checkin, for whatever reason.  I've -r MOZILLA_1_8_BRANCH it and all seems well.  Sorry for the false alarm :(
I'm going to work up a 1_5 branch version of this to make mento's life easier.
Flags: camino1.5.2?
mento, I assume you want to take a look at this since it's not exactly the same as 1.8 (no xcconfigs, 2 Info-Caminos, no FOX_APP stuff, Xcode 1.x project being the big ones).

Also, since I had to clean up after generated in this patch anyway, this also has the garbage dirs cleanup (bug 373280).

Happy version-bumping!
Attachment #282502 - Flags: superreview?(mark)
Comment on attachment 282502 [details] [diff] [review]
1_5 branch version

1_5 branch cvs removes are 

Info-Camino.plist
Info-CaminoStatic.plist
resources/application/all-camino.js
resources/localized/English.lproj/InfoPlist.strings
Attachment #282502 - Flags: superreview?(mark) → superreview+
Flags: camino1.5.2? → camino1.5.2+
Keywords: checkin-needed
Whiteboard: [1_5 branch cvs removes in comment 23]
Checked in the Camino 1.5 branch patch, this'll help us release 1.5.2.
Whiteboard: [1_5 branch cvs removes in comment 23] → [1_5 branch cvs removes in comment 23][camino-1.5.2]
Keywords: checkin-needed
Whiteboard: [1_5 branch cvs removes in comment 23][camino-1.5.2] → [camino-1.5.2]
You need to log in before you can comment on or make changes to this bug.