Closed Bug 955657 Opened 10 years ago Closed 10 years ago

Switch to using moz.build

Categories

(Instantbird Graveyard :: Other, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

References

Details

Attachments

(1 file, 4 obsolete files)

*** Original post on bio 2212 at 2013-10-10 18:29:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch WIP v2 (obsolete) — Splinter Review
*** Original post on bio 2212 as attmnt 2935 at 2013-10-10 18:29:00 UTC ***

See bug 955433 (bio 1997) for where this work was originally done.

There are issues building libpurple still, I assume.
Comment on attachment 8354711 [details] [diff] [review]
WIP v2

*** Original change on bio 2212 attmnt 2935 at 2013-10-10 18:41:48 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354711 - Flags: feedback?(florian)
*** Original post on bio 2212 at 2013-10-12 11:11:41 UTC ***

Comment on attachment 8354711 [details] [diff] [review] (bio-attmnt 2935)
WIP v2

Some feedback after only looking at the patch (not tried it yet) :

- the indent is strange in purple/moz.build
- there's an empty line at the end of a few moz.build files that looks like it has no reason to be there (instantbird/branding/*/moz.build instantbird/app/profile/moz.build chat/moz.build).
- you can drop all the MOZ_INCOMPLETE_EXTERNAL_LINKAGE stuff, it's specific to mailnews/. purplexpcom works correctly with external xpcom linkage. (instantbird/app.mozbuild and instantbird/moz.build)
- the lines you added to the top level moz.build file don't exist in comm-central, they are only inside the mozilla/ subfolder.
Attached patch WIP v3 (obsolete) — Splinter Review
*** Original post on bio 2212 as attmnt 2970 at 2013-10-19 22:30:00 UTC ***

This fixes the issues in comment 2, which I think Florian already fixed in his version.
Comment on attachment 8354711 [details] [diff] [review]
WIP v2

*** Original change on bio 2212 attmnt 2935 at 2013-10-19 22:30:37 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354711 - Attachment is obsolete: true
Attachment #8354711 - Flags: feedback?(florian)
Attached patch Interdiff (obsolete) — Splinter Review
*** Original post on bio 2212 as attmnt 2972 at 2013-10-20 22:08:00 UTC ***

Interdiff of the changes I made to make this work on my linux machine.
Attachment #8354753 - Flags: feedback?(clokep)
Comment on attachment 8354753 [details] [diff] [review]
Interdiff

*** Original change on bio 2212 attmnt 2972 at 2013-10-20 22:40:09 UTC ***

Looks pretty good, was this tried only on Linux or does it also work on Mac? I can try this on Windows if we're somewhat confident with it.
Attachment #8354753 - Flags: feedback?(clokep) → feedback+
*** Original post on bio 2212 at 2013-10-21 00:32:37 UTC ***

(In reply to comment #4)

> was this tried only on Linux or does it also work on Mac?

Tried only on Linux for now. I'm not too worried about platform differences, but I would like to try a clobber build (I've obviously done a lot of tinkering while I was debugging this, and I would like to check that nothing relied on remnants of a previous build :)).

What worries me though is that this build system doesn't seem very deterministic. Plenty of times, I had failures while building some libpurple files (the failures were caused by headers files of the same prpl that were not found; or by missing defines even though they are expected to be defined by the makefile...); and just trying again 'fixed' it.

> I can try this on Windows if we're somewhat confident with it.

Would be nice to try there before we check it in :).

We should also check |make package|.
*** Original post on bio 2212 at 2013-10-23 22:00:40 UTC ***

Comment on attachment 8354751 [details] [diff] [review] (bio-attmnt 2970)
WIP v3

>diff --git a/configure.in b/configure.in

> # --enable-application needs to find confvars.sh, build.mk, etc in the gived dir, so add ../
> # --external-source-dir is so the build system doesn't reject files outside known locations.
> # --disable-official-branding disables all checks for official branding, as we're doing this ourselves in our own repo
>-ac_configure_args="$_SUBDIR_CONFIG_ARGS --enable-application=../$MOZ_BUILD_APP --with-external-source-dir=$external_topsrcdir --disable-official-branding --with-branding=../$REAL_BRANDING_DIRECTORY"
>+ac_configure_args="$_SUBDIR_CONFIG_ARGS --enable-application=../$MOZ_BUILD_APP --with-external-source-dir=$_topsrcdir --disable-official-branding --with-branding=../$REAL_BRANDING_DIRECTORY"

This change seems to have been made by accident. It breaks the Windows build.
*** Original post on bio 2212 at 2013-10-23 22:17:28 UTC ***

(In reply to comment #5)
> (In reply to comment #4)

> We should also check |make package|.

Like I said yesterday, it's broken. The good news is I've just found the fix, and it's easy: you just need to port http://hg.mozilla.org/comm-central/rev/e5f81ee8ccf8
Attached patch Patch v1 (obsolete) — Splinter Review
*** Original post on bio 2212 as attmnt 2979 at 2013-10-24 10:31:00 UTC ***

This just takes my WIP, Florian's interdiff, fixes the external source dir issue, and then does that new c-c changeset over it.

I tested this on Windows and it passes configure, builds and packages. I did not test a clobber build.
Attachment #8354760 - Flags: review?(florian)
Comment on attachment 8354751 [details] [diff] [review]
WIP v3

*** Original change on bio 2212 attmnt 2970 at 2013-10-24 10:31:09 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354751 - Attachment is obsolete: true
Comment on attachment 8354753 [details] [diff] [review]
Interdiff

*** Original change on bio 2212 attmnt 2972 at 2013-10-24 10:31:09 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354753 - Attachment is obsolete: true
Comment on attachment 8354760 [details] [diff] [review]
Patch v1

*** Original change on bio 2212 attmnt 2979 at 2013-10-25 00:26:45 UTC ***

(In reply to comment #1)
> Comment on attachment 8354711 [details] [diff] [review] (bio-attmnt 2935) [details]

> - there's an empty line at the end of a few moz.build files that looks like it
> has no reason to be there (instantbird/branding/*/moz.build
> instantbird/app/profile/moz.build chat/moz.build).
> - you can drop all the MOZ_INCOMPLETE_EXTERNAL_LINKAGE stuff, it's specific to
> mailnews/. purplexpcom works correctly with external xpcom linkage.
> (instantbird/app.mozbuild and instantbird/moz.build)
> - the lines you added to the top level moz.build file don't exist in
> comm-central, they are only inside the mozilla/ subfolder.

These comments have not been addressed :(.
Attachment #8354760 - Flags: review?(florian) → review-
Attached patch Patch v2Splinter Review
*** Original post on bio 2212 as attmnt 2983 at 2013-10-25 02:08:00 UTC ***

Sorry about that, I had made these changes as a separate patch in my mq and then based it off an older version. :( This meets the review comments.
Attachment #8354764 - Flags: review?(florian)
Comment on attachment 8354760 [details] [diff] [review]
Patch v1

*** Original change on bio 2212 attmnt 2979 at 2013-10-25 02:08:22 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354760 - Attachment is obsolete: true
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment on attachment 8354764 [details] [diff] [review]
Patch v2

*** Original change on bio 2212 attmnt 2983 at 2013-10-25 10:42:21 UTC ***

Looks ready this time :-).
Attachment #8354764 - Flags: review?(florian) → review+
*** Original post on bio 2212 at 2013-10-25 20:34:55 UTC ***

http://hg.instantbird.org/instantbird/rev/a9e6354b9484

This commit had the original bug (bug 955433 (bio 1997)) I did this patch for. :( Oops.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Depends on: 955433
Resolution: --- → FIXED
Target Milestone: --- → 1.5
*** Original post on bio 2212 at 2013-10-26 20:10:50 UTC ***

Bustage fixes:
http://hg.instantbird.org/instantbird/rev/8a0955cee879
http://hg.instantbird.org/instantbird/rev/1a1646b1ab9a
*** Original post on bio 2212 at 2013-10-29 23:28:38 UTC ***

Add back some lines that were erroneously removed:
http://hg.instantbird.org/instantbird/rev/62e7c2a4074a
*** Original post on bio 2212 at 2013-11-03 13:24:30 UTC ***

Fix upload paths (lines that were erroneously removed):
http://hg.instantbird.org/instantbird/rev/2a59bed0ca18
*** Original post on bio 2212 at 2013-11-03 13:45:10 UTC ***

(In reply to comment #14)
> Add back some lines that were erroneously removed:
> http://hg.instantbird.org/instantbird/rev/62e7c2a4074a
This revision was lost during the server migration and is the same as that in comment #15.
You need to log in before you can comment on or make changes to this bug.