Closed Bug 463417 Opened 13 years ago Closed 12 years ago
import win32 nsinstall source into mozilla-central, build by default on win32
24.39 KB, patch
|Details | Diff | Splinter Review|
2.57 KB, patch
|Details | Diff | Splinter Review|
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?
The host would be able to compile the unixy nsinstall.c, though, wouldn't it?
Yes, and Neil says we already do in that case.
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #387684 - Flags: review?
Attachment #387684 - Flags: review? → review?(timeless)
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+
http://hg.mozilla.org/mozilla-central/rev/b66e1b78a27b http://hg.mozilla.org/mozilla-central/rev/265fc2d9d12d http://hg.mozilla.org/mozilla-central/rev/aa2308769c49 http://hg.mozilla.org/mozilla-central/rev/23e92be1601f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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?
Ah, I figured it out, it will pick it up from the $NSINSTALL_BIN env variable.
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
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]
You need to log in before you can comment on or make changes to this bug.