956609 - Land the IM front-end in comm-central

Status: RESOLVED FIXED

(Instantbird Graveyard :: Other, defect)

Description

[Patrick Cloke [:clokep]]

It would simplify landing of chat/ changes for Thunderbird and Instantbird if these two products existed in the same repository. This would avoid the need for the "Port chat/ changes bugs" (bug 842183, bug 842183, bug 812921, bug 799068, bug 776901 and bug 753807) which are very prone to cause errors. They also have the awkwardness of having to port a bunch of UI changes to go with backend changes all at once instead of handling them individually in each bug.

Comment 1

[Patrick Cloke [:clokep]]

Attachment: 1 - Copy instantbird/ to im/

Comment 2

[Patrick Cloke [:clokep]]

Attachment: 2 - Remove Makefiles from chat/

Comment 3

[Patrick Cloke [:clokep]]

Attachment: 3 - Revert moz.build files in chat/

Comment 4

[Patrick Cloke [:clokep]]

Attachment: 4 - Remove references to purple/

Comment 5

[Patrick Cloke [:clokep]]

Attachment: 5 - s/instantbird/im/

Comment 6

[Patrick Cloke [:clokep]]

Attachment: 6 - Port bug 870565

Comment 7

[Patrick Cloke [:clokep]]

Attachment: 7 - Fix JAR_MANIFESTS in moz.build files

Comment 8

[Patrick Cloke [:clokep]]

Attachment: 8 - Revert Makefile.in in chat/

Comment 9

[Patrick Cloke [:clokep]]

Attachment: 9 - s/core_abspath/$(abspath)/

Comment 10

[Patrick Cloke [:clokep]]

Attachment: 10 - Handle EXTRA_JS_MODULES in im/

Comment 11

[Patrick Cloke [:clokep]]

Attachment: 11 - Handle EXTRA_COMPONENTS in im/

Comment 12

[Patrick Cloke [:clokep]]

Attachment: 12 - Handle MODULE in im/

Comment 13

[Patrick Cloke [:clokep]]

Attachment: 13 - Handle XPIDLSRCS in im/

Comment 14

[Patrick Cloke [:clokep]]

Attachment: 14 - Handle all changes to mintrayr Makefile.in

Comment 15

[Patrick Cloke [:clokep]]

Attachment: 15 - Stop referencing preprocessor.py

Comment 16

[Patrick Cloke [:clokep]]

Attachment: 16 - Add Instantbird's 7zstub

Comment 17

[Patrick Cloke [:clokep]]

Attachment: 17 - Remove incomplete linkage stuff that Florian already asked me to remove a bunch of times

Comment 18

[Patrick Cloke [:clokep]]

Attachment: 18 - Port changes to app/Makefile.in

Comment 19

[Patrick Cloke [:clokep]]

Attachment: 19 - Fix package.manifest

Comment 20

[Patrick Cloke [:clokep]]

Attachment: 20 - Extra changes to get build/packaging to work on Mac OS X

Comment 21

[Patrick Cloke [:clokep]]

Comment on attachment 8355940 [details] [diff] [review]
1 - Copy instantbird/ to im/

The patch copying instantbird/ to im/ doesn't actually need review as I believe Florian wishes to hg convert the current instantbird repo in such a way as to get some of the history of the Instantbird repository. This just servers as a baseline for the other patches in the series.

Comment 22

[Florian Quèze [:florian]]

Comment on attachment 8355944 [details] [diff] [review]
5 - s/instantbird/im/

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

::: im/confvars.sh
@@ +14,5 @@
>  MOZ_APP_VERSION=`cat $MOZ_APP_VERSION_TXT`
>  INSTANTBIRD_VERSION=$MOZ_APP_VERSION
>  
> +MOZ_BRANDING_DIRECTORY=im/branding/nightly
> +MOZ_OFFICIAL_BRANDING_DIRECTORY=im/branding/release

Is our official branding going to im/branding/release, or other-licenses/branding/instantbird/ ?

Comment 23

[Florian Quèze [:florian]]

Comment on attachment 8355945 [details] [diff] [review]
6 - Port bug 870565

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

As a port of bug 870565 this is fine. In the future I wonder if we should move add_tier_dir('app', 'extensions') to after 'chat', so that we can build purple/ inside the extensions/ folder, after the chat idl files have already been processed.

Comment 24

[Florian Quèze [:florian]]

Comment on attachment 8355946 [details] [diff] [review]
7 - Fix JAR_MANIFESTS in moz.build files

The changes here are fine. There are similar JAR_MANIFEST changes inside attachment 8355942 [details] [diff] [review], I wonder if these 2 attachments should be merged. On the other hand, the chat vs im separation also makes some sense, so r+ anyway.

Comment 25

[Florian Quèze [:florian]]

Comment on attachment 8355948 [details] [diff] [review]
9 - s/core_abspath/$(abspath)/

Seems fine, do we know which Thunderbird changeset matches these changes?

Comment 26

[Florian Quèze [:florian]]

Comment on attachment 8355950 [details] [diff] [review]
11 - Handle EXTRA_COMPONENTS in im/

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

::: im/components/moz.build
@@ +18,5 @@
> +else:
> +    EXTRA_COMPONENTS += ['profileMigrator.js', 'profileMigrator.manifest']
> +
> +EXTRA_PP_COMPONENTS += [
> +	'ibCommandLineHandler.js',

I think we tried to have this line close to 'ibCommandLineHandler.manifest' before. Any reason for changing the order of these definitions?


This file has mixed tabs and spaces for the indent. How are the moz.build files usually indented?

Comment 27

[Florian Quèze [:florian]]

Comment on attachment 8355952 [details] [diff] [review]
13 - Handle XPIDLSRCS in im/

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

r+ but please be careful with the indentation here (I asked in a previous review comment to fix the indentation in this moz.build file.

Comment 28

[Florian Quèze [:florian]]

Comment on attachment 8355953 [details] [diff] [review]
14 - Handle all changes to mintrayr Makefile.in

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

Seems reasonable. One nit.

::: im/components/mintrayr/moz.build
@@ +19,5 @@
> +
> +LIBRARY_NAME = 'trayToolkit'
> +
> +XPIDL_SOURCES += [
> +	'trayIToolkit.idl',

Mixed tab/spaces in this file too.

Comment 29

[Florian Quèze [:florian]]

Comment on attachment 8355954 [details] [diff] [review]
15 - Stop referencing preprocessor.py

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

::: im/installer/windows/Makefile.in
@@ +60,3 @@
>  	$(PYTHON) $(MOZILLA_SRCDIR)/toolkit/mozapps/installer/windows/nsis/preprocess-locale.py \
>            --preprocess-locale $(MOZILLA_SRCDIR) \
> +	  $(call EXPAND_LOCALE_SRCDIR,im/locales)/installer $(AB_CD) $(CONFIG_DIR)

Wasn't this change intended to go in the s/instantbird/im/ patch?

@@ +70,3 @@
>  	$(PYTHON) $(MOZILLA_SRCDIR)/toolkit/mozapps/installer/windows/nsis/preprocess-locale.py \
>  	  --preprocess-locale $(MOZILLA_SRCDIR) \
> +	  $(call EXPAND_LOCALE_SRCDIR,im/locales)/installer $(AB_CD) $(CONFIG_DIR)

Same here.

Comment 30

[Florian Quèze [:florian]]

Comment on attachment 8355955 [details] [diff] [review]
16 - Add Instantbird's 7zstub

Is this an exact copy of the file currently in the Instantbird repository?

Comment 31

[Florian Quèze [:florian]]

Comment on attachment 8355957 [details] [diff] [review]
18 - Port changes to app/Makefile.in

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

::: im/app/Makefile.in
@@ -46,5 @@
> -
> -ifneq (,$(filter OS2 WINNT,$(OS_ARCH)))
> -PROGRAM = $(MOZ_APP_NAME)$(BIN_SUFFIX)
> -else
> -PROGRAM = $(MOZ_APP_NAME)-bin$(BIN_SUFFIX)

Thunderbird has a rule at http://hg.mozilla.org/comm-central/annotate/8198dbdebd96/mail/app/Makefile.in#l141 to replace this.

@@ -57,5 @@
>  ifneq (,$(filter OS2 WINNT,$(OS_ARCH)))
>  STUBNAME = $(MOZ_APP_NAME)$(BIN_SUFFIX)
>  else
>  STUBNAME = $(MOZ_APP_NAME)-bin$(BIN_SUFFIX)
>  endif

Why is this now done unconditionally? This is only for the case where a libxul sdk was provided at configure time, isn't it?

I think we need to look at all the stuff done in ifdef LIBXUL_SDK parts of the Thunderbird makefile.

Comment 32

[Florian Quèze [:florian]]

Comment on attachment 8355959 [details] [diff] [review]
20 - Extra changes to get build/packaging to work on Mac OS X

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

::: im/app/Makefile.in
@@ +295,5 @@
>  ifdef MOZ_DEBUG
>  MAC_APP_NAME := $(MAC_APP_NAME)Debug
>  endif
>  
> +tools:: $(PROGRAM)

This should be its own changeset, with a clear reference to the bug we are porting.

::: im/installer/package-manifest.in
@@ +145,5 @@
>  #ifdef XP_MACOSX
>  @BINPATH@/components/ibDockBadge.js
>  @BINPATH@/components/ibDockBadge.manifest
>  #else
> +@BINPATH@/components/components.manifest

This should be part of "19 - Fix package.manifest"

Comment 33

[Florian Quèze [:florian]]

Comment on attachment 8355958 [details] [diff] [review]
19 - Fix package.manifest

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

The title here isn't really correct (there's no package.manifest file).

See the review comment of "20 - Extra changes to get build/packaging to work on Mac OS X" for the reason for the r-.

Comment 34

[Florian Quèze [:florian]]

Comment on attachment 8355945 [details] [diff] [review]
6 - Port bug 870565

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

::: im/app.mozbuild
@@ +4,5 @@
>  
>  app_libxul_dirs = []
>  app_libxul_static_dirs = []
>  
> +include('../bridge/bridge.mozbuild')

Joshua pointed out that we don't want this line.

Comment 35

[Patrick Cloke [:clokep]]

Attachment: 1 & 16 - Copy instantbird/ to im/ v2

Comment 36

[Patrick Cloke [:clokep]]

Attachment: 2 & 3 & 8 - Remove/revert Makefile.in and revert moz.build in chat/ v2

Comment 37

[Patrick Cloke [:clokep]]

Attachment: 5 - s/instantbird/im/ v2

Comment 38

[Patrick Cloke [:clokep]]

Attachment: 6 - Port bug 870565 v2

Comment 39

[Patrick Cloke [:clokep]]

Attachment: 7 - Fix JAR_MANIFESTS in moz.build files v2

Comment 40

[Patrick Cloke [:clokep]]

Attachment: 9 - s/core_abspath/$(abspath)/ v2

Comment 41

[Patrick Cloke [:clokep]]

Attachment: 10 - Handle EXTRA_JS_MODULES in im/ v2

Comment 42

[Patrick Cloke [:clokep]]

Attachment: 11 - Handle EXTRA_COMPONENTS in im/ v2

Comment 43

[Patrick Cloke [:clokep]]

Attachment: 12 & 13 - Handle MODULE and XPIDLSRCS in im/ v2

Comment 44

[Patrick Cloke [:clokep]]

Attachment: 14 - Handle all changes to mintrayr Makefile.in v2

Comment 45

[Patrick Cloke [:clokep]]

Attachment: 15 - Stop referencing preprocessor.py v2

Comment 46

[Patrick Cloke [:clokep]]

Attachment: 18 - Port changes to app/Makefile.in v2

Comment 47

[Patrick Cloke [:clokep]]

Attachment: 19 - Fix package-manifest.in v2

Comment 48

[Patrick Cloke [:clokep]]

Attachment: 20 - Mac OS changes v2

Comment 49

[Patrick Cloke [:clokep]]

Response to your review comments. I probably should have split these up by patch. Sorry for doing it in a stupid way.

(In reply to Florian Quèze [:florian] [:flo] from comment #22)
> Comment on attachment 8355944 [details] [diff] [review]
> 5 - s/instantbird/im/
>
> Review of attachment 8355944 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: im/confvars.sh
> @@ +14,5 @@
> >  MOZ_APP_VERSION=`cat $MOZ_APP_VERSION_TXT`
> >  INSTANTBIRD_VERSION=$MOZ_APP_VERSION
> >
> > +MOZ_BRANDING_DIRECTORY=im/branding/nightly
> > +MOZ_OFFICIAL_BRANDING_DIRECTORY=im/branding/release
>
> Is our official branding going to im/branding/release, or
> other-licenses/branding/instantbird/ ?

I think we'll move this to other-licenses/branding/instantbird/ as part of the hg convert, so I'll do this in the first commit. I've updated this path to reflect this move.

(In reply to Florian Quèze [:florian] [:flo] from comment #23)
> Comment on attachment 8355945 [details] [diff] [review]
> 6 - Port bug 870565
>
> Review of attachment 8355945 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> As a port of bug 870565 this is fine. In the future I wonder if we should
> move add_tier_dir('app', 'extensions') to after 'chat', so that we can build
> purple/ inside the extensions/ folder, after the chat idl files have already
> been processed.
I've moved this. I did not include any

(In reply to Florian Quèze [:florian] [:flo] from comment #34)
> Comment on attachment 8355945 [details] [diff] [review]
> 6 - Port bug 870565
>
> Review of attachment 8355945 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: im/app.mozbuild
> @@ +4,5 @@
> >
> >  app_libxul_dirs = []
> >  app_libxul_static_dirs = []
> >
> > +include('../bridge/bridge.mozbuild')
>
> Joshua pointed out that we don't want this line.
I've removed this.

(In reply to Florian Quèze [:florian] [:flo] from comment #24)
> Comment on attachment 8355946 [details] [diff] [review]
> 7 - Fix JAR_MANIFESTS in moz.build files
>
> The changes here are fine. There are similar JAR_MANIFEST changes inside
> attachment 8355942 [details] [diff] [review], I wonder if these 2
> attachments should be merged. On the other hand, the chat vs im separation
> also makes some sense, so r+ anyway.

The ones in attachment 8355942 [details] [diff] [review] are automated hg rm / hg revert, the ones here were made by hand. They should be kept separate, I think.

(In reply to Florian Quèze [:florian] [:flo] from comment #25)
> Comment on attachment 8355948 [details] [diff] [review]
> 9 - s/core_abspath/$(abspath)/
>
> Seems fine, do we know which Thunderbird changeset matches these changes?

Bug 928966, I've added that to the current commit message.

(In reply to Florian Quèze [:florian] [:flo] from comment #26)
> Comment on attachment 8355950 [details] [diff] [review]
> 11 - Handle EXTRA_COMPONENTS in im/
>
> Review of attachment 8355950 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: im/components/moz.build
> @@ +18,5 @@
> > +else:
> > +    EXTRA_COMPONENTS += ['profileMigrator.js', 'profileMigrator.manifest']
> > +
> > +EXTRA_PP_COMPONENTS += [
> > +	'ibCommandLineHandler.js',
>
> I think we tried to have this line close to 'ibCommandLineHandler.manifest'
> before. Any reason for changing the order of these definitions?

No real reason, I've moved it back.

> This file has mixed tabs and spaces for the indent. How are the moz.build
> files usually indented?

Spaces, I've checked the files and they SHOULD have all spaces now. (As also mentioned in comment #27 and comment #28.)

(In reply to Florian Quèze [:florian] [:flo] from comment #29)
> Comment on attachment 8355954 [details] [diff] [review]
> 15 - Stop referencing preprocessor.py
>
> Review of attachment 8355954 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: im/installer/windows/Makefile.in
> @@ +60,3 @@
> >  	$(PYTHON) $(MOZILLA_SRCDIR)/toolkit/mozapps/installer/windows/nsis/preprocess-locale.py \
> >            --preprocess-locale $(MOZILLA_SRCDIR) \
> > +	  $(call EXPAND_LOCALE_SRCDIR,im/locales)/installer $(AB_CD) $(CONFIG_DIR)
>
> Wasn't this change intended to go in the s/instantbird/im/ patch?

Yes (for both of them), I've moved them.

(In reply to Florian Quèze [:florian] [:flo] from comment #30)
> Comment on attachment 8355955 [details] [diff] [review]
> 16 - Add Instantbird's 7zstub
>
> Is this an exact copy of the file currently in the Instantbird repository?

I've moved this into the first patch. I'm assuming this will be done as part of the convert.

(In reply to Florian Quèze [:florian] [:flo] from comment #31)
> Comment on attachment 8355957 [details] [diff] [review]
> 18 - Port changes to app/Makefile.in
>
> Review of attachment 8355957 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: im/app/Makefile.in
> @@ -46,5 @@
> > -
> > -ifneq (,$(filter OS2 WINNT,$(OS_ARCH)))
> > -PROGRAM = $(MOZ_APP_NAME)$(BIN_SUFFIX)
> > -else
> > -PROGRAM = $(MOZ_APP_NAME)-bin$(BIN_SUFFIX)
>
> Thunderbird has a rule at
> http://hg.mozilla.org/comm-central/annotate/8198dbdebd96/mail/app/Makefile.
> in#l141 to replace this.

OK, I added this back in.

> @@ -57,5 @@
> >  ifneq (,$(filter OS2 WINNT,$(OS_ARCH)))
> >  STUBNAME = $(MOZ_APP_NAME)$(BIN_SUFFIX)
> >  else
> >  STUBNAME = $(MOZ_APP_NAME)-bin$(BIN_SUFFIX)
> >  endif
>
> Why is this now done unconditionally? This is only for the case where a
> libxul sdk was provided at configure time, isn't it?
>
> I think we need to look at all the stuff done in ifdef LIBXUL_SDK parts of
> the Thunderbird makefile.

This file is extremely different than mail/app/Makefile.in. I might need a bit more of help looking at this file.

(In reply to Florian Quèze [:florian] [:flo] from comment #32)
> Comment on attachment 8355959 [details] [diff] [review]
> 20 - Extra changes to get build/packaging to work on Mac OS X
>
> Review of attachment 8355959 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: im/app/Makefile.in
> @@ +295,5 @@
> >  ifdef MOZ_DEBUG
> >  MAC_APP_NAME := $(MAC_APP_NAME)Debug
> >  endif
> >
> > +tools:: $(PROGRAM)
>
> This should be its own changeset, with a clear reference to the bug we are
> porting.

Done (bug 912292, for the record).

> ::: im/installer/package-manifest.in
> @@ +145,5 @@
> >  #ifdef XP_MACOSX
> >  @BINPATH@/components/ibDockBadge.js
> >  @BINPATH@/components/ibDockBadge.manifest
> >  #else
> > +@BINPATH@/components/components.manifest
>
> This should be part of "19 - Fix package.manifest"

Done.

(In reply to Florian Quèze [:florian] [:flo] from comment #33)
> Comment on attachment 8355958 [details] [diff] [review]
> 19 - Fix package.manifest
>
> Review of attachment 8355958 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> The title here isn't really correct (there's no package.manifest file).

I'll fix this for the next uploaded patch.

Comment 50

[Florian Quèze [:florian]]

Comment on attachment 8357542 [details] [diff] [review]
2 & 3 & 8 - Remove/revert Makefile.in and revert moz.build in chat/ v2

This will need to be updated once bug 957918 lands. There's no way we can add "protocols/vkontakte" again for Instantbird ;).

Comment 51

[Florian Quèze [:florian]]

Comment on attachment 8357547 [details] [diff] [review]
6 - Port bug 870565 v2

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

::: im/app.mozbuild
@@ +12,3 @@
>  
> +add_tier_dir('app', '../chat')
> +add_tier_dir('app', '../im')

im/app needs to be the last folder we build, so if we are hoping that purple/ will be built by the extensions folder, 'im' needs to be after 'extensions'.

Comment 52

[Florian Quèze [:florian]]

Comment on attachment 8357556 [details] [diff] [review]
18 - Port changes to app/Makefile.in v2

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

Seems good otherwise, but I think I would be more confident if we had verified that a build works on all OSes with this change applied.

::: im/app/Makefile.in
@@ +128,5 @@
> +ifneq (,$(filter-out OS2 WINNT,$(OS_ARCH)))
> +libs::
> +	cp -p $(MOZ_APP_NAME)$(BIN_SUFFIX) $(DIST)/bin/$(MOZ_APP_NAME)-bin$(BIN_SUFFIX)
> +
> +GARBAGE += $(addprefix $(DIST)/bin/defaults/pref/, all.js all-instantbird.js channel-prefs.js chat.js)

There's no chat.js file, I think you wanted chat-prefs.js.

Comment 53

[Patrick Cloke [:clokep]]

Attachment: 2 & 3 & 8 - Remove/revert Makefile.in and revert moz.build in chat/ v3

(In reply to Florian Quèze [:florian] [:flo] from comment #50)
> Comment on attachment 8357542 [details] [diff] [review]
> 2 & 3 & 8 - Remove/revert Makefile.in and revert moz.build in chat/ v2
> 
> This will need to be updated once bug 957918 lands. There's no way we can
> add "protocols/vkontakte" again for Instantbird ;).

Done.

Comment 54

[Patrick Cloke [:clokep]]

Attachment: 6 - Port bug 870565 v3

(In reply to Florian Quèze [:florian] [:flo] from comment #51)
> Comment on attachment 8357547 [details] [diff] [review]
> 6 - Port bug 870565 v2
> 
> Review of attachment 8357547 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: im/app.mozbuild
> @@ +12,3 @@
> >  
> > +add_tier_dir('app', '../chat')
> > +add_tier_dir('app', '../im')
> 
> im/app needs to be the last folder we build, so if we are hoping that
> purple/ will be built by the extensions folder, 'im' needs to be after
> 'extensions'.

I added the extensions part between chat/ and im/.

Comment 55

[Patrick Cloke [:clokep]]

Attachment: 18 - Port changes to app/Makefile.in v3

(In reply to Florian Quèze [:florian] [:flo] from comment #52)
> Comment on attachment 8357556 [details] [diff] [review]
> 18 - Port changes to app/Makefile.in v2
> 
> Review of attachment 8357556 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Seems good otherwise, but I think I would be more confident if we had
> verified that a build works on all OSes with this change applied.
> 
> ::: im/app/Makefile.in
> @@ +128,5 @@
> > +ifneq (,$(filter-out OS2 WINNT,$(OS_ARCH)))
> > +libs::
> > +	cp -p $(MOZ_APP_NAME)$(BIN_SUFFIX) $(DIST)/bin/$(MOZ_APP_NAME)-bin$(BIN_SUFFIX)
> > +
> > +GARBAGE += $(addprefix $(DIST)/bin/defaults/pref/, all.js all-instantbird.js channel-prefs.js chat.js)
> 
> There's no chat.js file, I think you wanted chat-prefs.js.

I changed this. I'll see if I can get a try build working?

Comment 56

[Patrick Cloke [:clokep]]

Attachment: 2 & 3 & - Remove/revert Makefile.in and revert moz.build in chat/ v4

I did something really funky in my last patch and I have no idea what happened, but it's definitely wrong. This patch essentially becomes a no-op after the latest ib->c-c merge, besides removing the Makefile.in files.

Comment 57

[Patrick Cloke [:clokep]]

Comment on attachment 8358181 [details] [diff] [review]
2 & 3 & 8 - Remove/revert Makefile.in and revert moz.build in chat/ v3

This is the proper patch, turns out I was diffing against a repo of IB with patches applied to it.

Comment 58

[Florian Quèze [:florian]]

Comment on attachment 8358183 [details] [diff] [review]
18 - Port changes to app/Makefile.in v3

(In reply to Florian Quèze [:florian] [:flo] from comment #52)
> Comment on attachment 8357556 [details] [diff] [review]
> 18 - Port changes to app/Makefile.in v2

> Seems good otherwise, but I think I would be more confident if we had
> verified that a build works on all OSes with this change applied.

Actually, it doesn't work on Mac.

> ::: im/app/Makefile.in
> @@ +128,5 @@
> > +ifneq (,$(filter-out OS2 WINNT,$(OS_ARCH)))
> > +libs::
> > +	cp -p $(MOZ_APP_NAME)$(BIN_SUFFIX) $(DIST)/bin/$(MOZ_APP_NAME)-bin$(BIN_SUFFIX)

This rule needs to be after the inclusion of rules.mk

Comment 59

[Florian Quèze [:florian]]

Attachment: 18 - Port changes to app/Makefile.in v4

New patch addressing comment 58. (r+'ing as it's clokep's patch with my review comment addressed)

Comment 60

[Florian Quèze [:florian]]

https://hg.mozilla.org/comm-central/pushloghtml?changeset=f81a23bfefcd

Comment 61

[Florian Quèze [:florian]]

Fixed path to the Halloween branding: https://hg.mozilla.org/comm-central/rev/d59eb3bf90e9