import win32 nsinstall source into mozilla-central, build by default on win32

RESOLVED FIXED in mozilla1.9.2a1

Status

Firefox Build System
General
RESOLVED FIXED
10 years ago
3 months ago

People

(Reporter: ted, Assigned: Benjamin Smedberg)

Tracking

Trunk
mozilla1.9.2a1
x86
Windows XP
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
We currently ship nsinstall.exe with MozillaBuild, and the win32 version isn't in mozilla-central. This is probably just for historical reasons. The down side is that if we find a bug in nsinstall, like bug 463411, it's very hard to fix. I don't see any reason we shouldn't just import the source and build it by default on win32. We will obviously have to account for the mingw cross-compile case, since the host won't be able to compile the native nsinstall there.

cls: do you know of any other pitfalls we might hit here?
(Assignee)

Comment 1

10 years ago
The host would be able to compile the unixy nsinstall.c, though, wouldn't it?
(Reporter)

Comment 2

10 years ago
Yes, and Neil says we already do in that case.
(Assignee)

Comment 3

9 years ago
Created attachment 387684 [details] [diff] [review]
Build nsinstall.exe as part of the builds, rev. 1
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #387684 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #387684 - Flags: review? → review?(timeless)

Comment 4

9 years ago
Comment on attachment 387684 [details] [diff] [review]
Build nsinstall.exe as part of the builds, rev. 1

btw, i'm perfectly fine with you checking in a patch series (in fact, i think it's a good idea), the original and the one for review comments.

That way someone can more easily compare with the original.

Among other things, nsinstall's code style /generally/ includes a space between function and ( arguments ) ( and inside parens too ), which isn't typical for modern Gecko style.

>diff --git a/config/nsinstall_win.c b/config/nsinstall_win.c

>+ * The nsinstall command for Win32

most of this block is legacy, and could go for a good spring cleaning :)

>+/* changes all forward slashes in token to back slashes */

google: did you mean backslashes :)

>+void changeForwardSlashesTpBackSlashes ( wchar_t *arg )

>+int wmain(int argc, wchar_t *argv[ ])
over-indented:
>+        return shellNsinstall ( argv + 1 );

>+shellNsinstall (wchar_t **pArgv)

tabs :(
>+    int retVal = 0;		/* exit status */
>+    int retVal = 0;			/* assume valid return */

please fix the spelling :)
>+            /* check if directory alreay exists */
Attachment #387684 - Flags: review?(timeless) → review+
Depends on: 505289

Updated

9 years ago
Depends on: 505574

Comment 6

9 years ago
Comment on attachment 387684 [details] [diff] [review]
Build nsinstall.exe as part of the builds, rev. 1

>-MOZ_PATH_PROGS(NSINSTALL_BIN, nsinstall )
Fortunately comm-central's configure.in still autodetects my custom nsinstall in the path... if I didn't have that luxury would it still be possible for me to use my custom version?

Comment 7

9 years ago
Ah, I figured it out, it will pick it up from the $NSINSTALL_BIN env variable.

Updated

9 years ago
Blocks: 357052
Target Milestone: --- → mozilla1.9.2b1
Target Milestone: mozilla1.9.2b1 → mozilla1.9.2a1
Fwiw, (I assume it was from a repository sync')
mozilla1.9.3a1:
http://hg.mozilla.org/mozilla-central/rev/b5822dea95a4
import the nsinstall.exe source from CVS buildtools repository: make it capable of copying files with very long names, build it by default on Windows hosts, and stop using the moztools version
Created attachment 428638 [details] [diff] [review]
(Bv1-CC) Copy (the useful part of) it to comm-central, Copy bug 505574 too
[Checkin: Comment 10]
Attachment #428638 - Flags: review?(bugspam.Callek)

Updated

8 years ago
Attachment #428638 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 428638 [details] [diff] [review]
(Bv1-CC) Copy (the useful part of) it to comm-central, Copy bug 505574 too
[Checkin: Comment 10]


http://hg.mozilla.org/comm-central/rev/af3aee23f01e
Attachment #428638 - Attachment description: (Bv1-CC) Copy (the useful part of) it to comm-central, Copy bug 505574 too → (Bv1-CC) Copy (the useful part of) it to comm-central, Copy bug 505574 too [Checkin: Comment 10]

Updated

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