Status

defect
RESOLVED FIXED
9 years ago
a year ago

People

(Reporter: Mitch, Assigned: Mitch)

Tracking

Trunk
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Assignee

Description

9 years ago
Follow-up to bug 461444.
Assignee

Comment 1

9 years ago
Attachment #423528 - Flags: review?(ted.mielczarek)
It looks like the renaming of the files under extensions didn't retain history... perhaps not using the standard hg push without extensions (e.g. qimport, or whatever) will retain the history properly. I would really appreciate keeping the history on these files.
Yeah, just had this discussion with Mossop on IRC. Mitch's patch had the renames, and I qimported it directly, and at one point the renames were correct locally, but perhaps a rebase or something lost that data. :-(
Assignee

Updated

9 years ago
Attachment #423528 - Flags: review?(dtownsend)
Attachment #423528 - Flags: review?(dtownsend) → review?(robert.bugzilla)
Comment on attachment 423528 [details] [diff] [review]
toolkit/mozapps/update (checked in)

>diff --git a/toolkit/mozapps/installer/wince/Makefile.in b/toolkit/mozapps/installer/wince/Makefile.in
>--- a/toolkit/mozapps/installer/wince/Makefile.in
>+++ b/toolkit/mozapps/installer/wince/Makefile.in
>@@ -40,27 +40,27 @@ topsrcdir = @top_srcdir@
> srcdir    = @srcdir@
> VPATH     = @srcdir@
> 
> include $(DEPTH)/config/autoconf.mk
> 
> export NO_SHUNT = 1
> USE_STATIC_LIBS = 1
> 
>-PROGRAM   = xulrunner-stub-installer.sfx
>+PROGRAM = xulrunner-stub-installer.sfx
> 
>-REQUIRES  = lib7z
>+REQUIRES = lib7z

Detabification is fine, but if you're going to go re-indenting all of these lines, you should make sure it's ok with Rob.

Looks fine otherwise.
Attachment #423528 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 423528 [details] [diff] [review]
toolkit/mozapps/update (checked in)

I'm fine with the detabification... it is just leftover from what used to be the standard.
Attachment #423528 - Flags: review?(robert.bugzilla) → review+
Also, please verify that the history isn't lost after checkin and fix if it isn't.
I can't compile the bsdiff tool (other-licenses/bsdiff, required for making partial update patches) since this patch landed. The preceding changeset (37928=>cf63181a4e51) works fine.

Here's the error output: http://pastebin.mozilla.org/701304
Blocks: 535369
Assignee

Comment 9

9 years ago
Fix for other-licenses/bsdiff/Makefile.in. Comment-only fixes in xpcom/glue and toolkit/content/license.html.
Attachment #425613 - Flags: review?(ted.mielczarek)
Comment on attachment 425613 [details] [diff] [review]
toolkit/mozapps/update fix (checked in)

Looks good and thanks... I should have caught that.

Sorry about that coop. I'll get this landed soon
Attachment #425613 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 425613 [details] [diff] [review]
toolkit/mozapps/update fix (checked in)

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/2d873df39b6a
Attachment #425613 - Attachment description: toolkit/mozapps/update fix → toolkit/mozapps/update fix (checked in)
Attachment #423528 - Attachment description: toolkit/mozapps/update → toolkit/mozapps/update (checked in)
(In reply to comment #10)
> Sorry about that coop. I'll get this landed soon

Fix WFM too. Thanks.
Depends on: 545257
Assignee

Comment 13

9 years ago
Posted patch netwerk (obsolete) — Splinter Review
This patch passes TryServer.
Attachment #444521 - Flags: review?(cbiesinger)
Attachment #444521 - Flags: review?(cbiesinger) → review+
Comment on attachment 444521 [details] [diff] [review]
netwerk

For HTTP at least, you forgot to copy gagan's name from the contributor line, and for cookie you forgot to copy IBM. Looks good otherwise, thanks.
Assignee

Comment 15

9 years ago
Posted patch netwerk (v2)Splinter Review
Attachment #444521 - Attachment is obsolete: true
Attachment #448708 - Flags: review?(cbiesinger)
Comment on attachment 448708 [details] [diff] [review]
netwerk (v2)

I'm not going to read this all again. assuming the only change is the added contributor lines, r=biesi
Attachment #448708 - Flags: review?(cbiesinger) → review+
Assignee

Updated

9 years ago
Keywords: checkin-needed
So, uhm, the landing of this in http://hg.mozilla.org/mozilla-central/rev/d8dc49d5bd60 seems to have clobbered Hg history for everything touched in /netwerk. :-( Same as happened before in comment 2.

Is there a way to fix this? That sucks.
(Ugh, I see the same thing having happened to files I "know" I hg renamed in previous commits... Is something broken with Hg/mq wrt to renames?)
(Ah-ha, Gavin pointed out that "hg log -f foo.cpp" shows that history is still there; even though hgweb and "hg log foo.cpp" cut it off before the rename. Bug on file is http://mercurial.selenic.com/bts/issue1576, which means the problem noted in comment 2 is the same issue.)

Sorry for the bugspam. :-/
Um, I see no evidence that this patch was actually landed as file renames; it looks like what got landed was a bunch of adds and removes, which is a really serious mistake.
Well after a brief discussion with David on IRC, it appears I had an old[er] HG version, with bugs. I rebased before I pushed, and rebasing in my HG version breaks renames :/

Anyway, the patch with broken renames was pushed as http://hg.mozilla.org/mozilla-central/rev/d8dc49d5bd60
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
I've relanded the patch with correct renames as <http://hg.mozilla.org/mozilla-central/rev/2d90590dabe6>.  However, until hg annotate is fixed, the life of people who want to annotate netwerk source files is harder than necessary.

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.