Closed Bug 454712 Opened 11 years ago Closed 11 years ago

WinCE tools need better command line parsing

Categories

(Firefox Build System :: General, defect)

x86
Windows XP
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wolfe, Assigned: wolfe)

References

Details

(Keywords: mobile)

Attachments

(2 files, 1 obsolete file)

For compiling SECURITY module, wince tools (arm-wince-gcc.exe, etc.) are being asked to compile from a different directory than the current directory, and place the output of the compilation into a third directory.  

All of these other directories are fully-qualified MingW32 pathnames, starting with (typically) "/c/..." -- which need to be converted into "c:/..."

Also, fully qualified pathnames are being tacked on to the end of a command line argument (such as "-Fo/c/..."), and these fully qualified pathnames need to be converted as well.

This attachment is a patch to update the "convert_args" function to handle these extra cases.  Not the prettiest code - but functional and easy to understand for a newbie trying to figure out what is going on inside the WinCE building tools.
Attachment #338006 - Flags: review?(doug.turner)
Attachment #338006 - Flags: review?(doug.turner) → review-
Comment on attachment 338006 [details] [diff] [review]
WinCE Tools Compiling Pathname Parsing Fix

the spacing is very off in this diff:

  {
+        char *offset;
+
    args_out[i] = args_in[i];



lets be consistent with the spacing;  feel free to go to town.  no tabs 2 or 4 spaces per indent.

Which version of the tools are we going to build with?  Is vs9 the future?  if so, can we drop vs8?

send me another patch w/ the spacing cleanup.  also lets talk about dropping vs8.
Actual Patch file that would have to be applied to mozilla-central, cleaned up with NO tabs and 2 spaces per indentation level.
Attachment #338175 - Flags: review?(doug.turner)
Comment on attachment 338006 [details] [diff] [review]
WinCE Tools Compiling Pathname Parsing Fix

obsoleted by next attachment -- just fogot to check the darn "obsoletes xxxx" checkbox.
Attachment #338006 - Attachment is obsolete: true
Cleaned up version of the real patch file attachment above.

Removed spurious line spacing change indications from these diffs, shuffled around removed lines to make changes more obvious.
Attachment #338178 - Flags: review?(doug.turner)
Well, VS8 is free and VS9 requires developers to pay Microsoft some money.  So far, we only know that the VS8 emulator has flaws - but we have no indication that the executable code compiled by VS8 is flawed.

So I would hesitate to drop VS8 support for WinMobile until (at least) we actually get a working XULRunner.exe

Especially since keeping VS8 support requires no work, but removing VS8 support is a little effort.
Attachment #338178 - Attachment mime type: application/octet-stream → text/plain
patch 338175 doesn't apply cleanly.  also, there is no vs9ppc2003arm directory in m-c.
Assignee: nobody → wolfe
I am not sure why the dependency on bug 444485 [Make WinMobile Build work with Visual Studio 2008 (VS9)] disappeared from this bug, but I am adding back the dependency.

Need to have already applied the attached patch for VS9, since this bug modifies the VS9 and VS8 shunt tools directories.
Depends on: 444485
Comment on attachment 338178 [details]
Beautifully hand-stitched patch file, derived from previous attachment - and modified to remove mention when line only changed by indentation level

the spacing is still messed up in toolspath.h.  also, lets change the drive letter name to be "c" to be consistent.
Attachment #338178 - Flags: review?(doug.turner) → review+
changeset:   19523:e9959c463a54
tag:         tip
user:        Doug Turner <dougt@meer.net>
date:        Mon Sep 22 22:19:53 2008 +0100
summary:     Bug 454712 - WinCE tools need better command line parsing. patch by wolfe. r=dougt.  npodb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #338175 - Flags: review?(doug.turner) → review-
Component: General → Build Config
Product: Fennec → Core
QA Contact: general → build-config
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.