2010/4/15 Nguyễn Thái Ngọc Duy <[email protected]>:
> Hi,
>
> This is something I think good enough to go upstream. The rest of my
> work is on:
>
> git://github.com/pclouds/busybox-w32.git wip
>
> Caveat: crappy, constantly rebased stuff as it's work in progress.
> However it may tell where this port leads to. If we don't count
> regex.c there are about 3k more lines to go.
>
>
> Nguyễn Thái Ngọc Duy (39):
>  ar: do not filter listing if there is no extra argument
>
> This is a regression, not related to Windows port at all.

Therefore such things should be submitted separately.

>  Config.in: add target platform selection
>  Config.in: add target platform WIN32

This looks like a correct approach for this.
Let's choose a good name for it. TARGET
often assumes architecture, maybe we should call it
TARGET_PLATFORM? Regarding WIN32 part, since there
are Cygwin-like POSIX-like emulation layers, WIN32
might be  ambiguous. Maybe NATIVE_WIN[DOWS]?

OTOH, TARGET_PLATFORM_NATIVE_WIN is a bit long...
but for now, it's the name which looks best to me.

>  Config.in: disable all commmands when TARGET_WIN32 is selected
>  win32: Refuse to build on Windows/MinGW unless TARGET_WIN32 is
>    selected

 menu "Runit Utilities"
+       depends on !TARGET_WIN32

I think a positive form, "depends on TARGET_PLATFORM_POSIX",
is better - you do not need to touch it for a gazillion of
other weird platforms it does not run on (if you ever add them).
Also, at this step, add it to every applet, not to applet groups.
The patch will be bigger, but further pathes will be smaller,
since they will only need to touch one entry whenever you make one
more applet to work on WIN - you add " || TARGET_PLATFORM_NATIVE_WIN"
for that applet.

>  win32: add missing system headers

This should not be added yet, see below.

>  platform.h: support MinGW port
>  win32: add termios.h

This should not be added yet, see below.

>  win32: add mingw.h
>  libbb.h: support MinGW port
>  win32: platform.h: add bswap_xx()
>  libbb: exclude files that will not compile on Windows

You are doing it wrongly. Instead, go to libbb/Kbuild
and replace

lib-y += get_console.o

with

lib-(TARGET_PLATFORM_POSIX) += get_console.o

>  libbb: update messages.c to support Windows
>  win32: Import fnmatch source

Starting from this, you have mostly things which definitely should not
go in yet.
First batch of patches should ONLY enable building on WIN,
even if the result can build only "true", "false" and "basename" applets.
(Not that it IS that bad, I think you will have more than that).
Let's merge things up to this intermediate point,
and then continue.

>  win32: set binary I/O mode by default
>  win32: add sleep()
>  win32: add mkstemp()
>  win32: add gettimeofday()
>  win32: add pipe()
>  win32: add gmtime_r()
>  win32: add localtime_r()
>  win32: add getpwuid()
>  win32: add signal routines and SIGALRM support
>  win32: add function to map windows errors to posix ones
>  win32: add link()
>  win32: add strsep()
>  win32: add realpath()
>  win32: add get_busybox_exec_path(), which is bb_busybox_exec_path
>  win32: add mkdir()
>  win32: add waitpid()
>  win32: add fcntl()
>  win32: add poll()
>  win32: add getenv(), setenv(), unsetenv() and clearenv()
>  Makefile: support building executable with extension
>  Makefile: support building on Windows using MinGW compiler
>  Add README.win32

After first part will be merged, next patchset should add a small set
of compat glue, and WIN-enable a few applets which are becoming buildable
because of that glue. This way, the patchsets will be easier to understand
and review.

In general, looks acceptable. There were some good comments
from other people, please take those which you agree with into account
and send a 1st patchset for merging when you have it.
-- 
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to