[Radiance-dev] Radiance compiled with mingw gcc (windows)

[Georg Mischler]

Greg Ward wrote:

> Francesco wrote:
>> I've finally started looking at rtcontrib to replace my own scripts to
>> calculate daylight coefficients, and need to use it with windows
>> (unfortunately!) and without cygwin, so I've been "forced" to try
>> the mingw
>> (http://www.mingw.org/) version of gcc to compile Radiance.
>
> Is this the target compiler we had in mind?  I don't know anything
> about what's available under Windows, if schorsch (Georg Mischler)
> meant for people to use the more common (and nasty) Visual C
> standard.

There's no target compiler per se. Compiler specific stuff is
defined in the platform/*.cfg files, and the SCons magic will
(hopefully) figure out which one to use.

The only difficulty here is that while we're compiling and
linking to the standard Windows libraries (apparently), the
gcc in mingw expects other command line arguments than the
VC compiler. We should create a seperate platform/mingw.cfg with
the parameters found. Francesco: Did you find any easy way to
tell the difference from within SCons/Python? We already have a
seperate *.cfg file for normal cygwin (using posix libraries),
but I don't remember how we figured out when to use that.

For the time being, you could try adding the following special
case to build_utils/load_plat.py (which should work, even if it's
not really pretty). I already checked in this change so it should
be in todays HEAD dump:

def load_plat(env, args, platform=None):
	for k,v in env.items(): print k,v
	if os.name == 'posix':
		POSIX_setup(env)
	if platform == None: # override
		p = sys.platform
	else: p = platform
	if p == 'win32' and 'gcc' in env['TOOLS']: # special case
		p = 'mingw'
	pl = []
	print 'Detected platform "%s" (%s).' % (sys.platform, os.name)
	[etc...]


Printing all items of env might produce better clues. Also, what
does mingw put in os.name and sys.platform?


>> and had to manually add some extra RAD_COMPAT inside a couple
>> of SConstruct files (maybe somebody can suggest a better
>> CPPDEFINES line ...).

What were the RAD_COMPAT entries?


>> As for the source files:
>>
>> 1.
>> I had to redefine kill() inside rad.c and ranimate.c
>> using "RT_PID pid" instead of "int pid":
>> int
>> kill(RT_PID pid, int sig) /* win|unix_process.c should also wait
>> and kill */
>> {
>>     return 0;
>> }
>
> This seems a safe enough change, though rtprocess.h, where RT_PID is
> defined, is not currently included in ranimate.c and will need to be
> added.

Yes, the change as such is safe, although there were a few more
places where int needed to be replaced by RT_PID (and should have
been pid_t to start with). Of course, the real fix would be to
use our own process abstraction open_process() and friends
instead of fork() and exec(). In other words: both programs may
compile on Windows with those changes (now in CVS), but will not
necessarily work as expected yet.


>> Also, inside win_process.c for the close_process() function:
>> was -----------------> int win_kill(pid, 0);
>> i've changed it to --> win_kill(pid, 0);
>
> I don't know if this affects compatibility under other compilers, so
> I'll leave it to schorsch.

win_kill() is a drop-in replacement for posix kill(), so it
must return an int for status information (even if it doesn't
actually do anything useful with it right now).


>> 2.
>> The mingw version of signal.h doesn't have a definition for
>> SIGALRM, so
>> killpersist() inside mkillum.c needs to be changed:
>> It is like this:
>> if (fscanf(fp, "%*s %d", &pid) != 1 || kill(pid, SIGALRM) < 0)
>> but I've used SIGTERM instead of SIGALRM, don't know if this make
>> much sense ...
>
> The only difference this will make is that the waiting rtrace process
> will report an error instead of going quietly.

I'm not sure if I understand this right now (but then, I just
returned from Mongolia, so don't expect too much).


>> 3.
>> rtcontrib was calling rtrace with a wrong command line. It was
>> something like
>> this:
>>
>> c:\radiance\bin/rtrace.exe rtrace [args ...]
>>
>> After some little searching I used this quick workaround inside
>> win_process.c
>>
>>             cmdpath = getpath(av[0], getenv("PATH"), X_OK);
>> added by me -->    av[0] = "";
>>             cmdstr = quoted_cmdline(cmdpath, av);
>>
>> that may have broken something somewhere else, so probably
>> a better solution would be to do some extra checks inside the
>> _WIN32 section
>> of getpath.c.
>>
>> I am interested only in rtcontrib right now, so I haven't checked
>> if other programs that call open_process() are still working or not.
>
> win_process.c is all schorsch's work, so he'll have to respond to
> this one as well.

I don't think your change breaks anything, but it's still not
the nice thing to do. Instead of feeding an empty string (which
results in a double width space in the command line) just pass
only the remainder of the argument list to quoted_cmdline():

  cmdstr = quoted_cmdline(cmdpath, av+1);

(Now in CVS)
I think this is save, because the first item will always be
the name of the command, and if there are no arguments, then
the second item will be NULL, which quoted_cmdline() should
handle correctly (I don't have a Windows system at hand right
now, though, so it's up to you to try...).


>> 4.
>> Mingw doesn't have fseeko(), and I replaced it with fseek() inside
>> rtcontrib.c.
>> This seems to have broken the file output, that is working well
>> with both
>> cygwin and linux. Is there an easy workaround for this? I don't
>> have very
>> much experience to figure it out myself. Output to stdout now works
>> fine ...
> The original call:
>
>      if (fseeko((FILE *)e->data, *(off_t *)p, SEEK_CUR) < 0) {
>
> could be replaced by:
>
>      if (fseek((FILE *)e->data, *(off_t *)p, SEEK_CUR) < 0) {
>
> or even -Dfseeko=fseek on the compile line and it should work for
> files less than 2 GB in size.  I assume that off_t is defined in your
> system, otherwise you would have had other errors.  If it isn't
> defined as "long", then you might need another cast, like so:
>
>      if (fseek((FILE *)e->data, (long)*(off_t *)p, SEEK_CUR) < 0) {
>
> Once we find a working substitute, we can add the appropriate macro
> to platform.h.

Microsoft only supports long files in the low level _lseeki64(), but
not in the stream routine(s). In the short term, -Dfseeko=fseek
looks like a reasonable workaround.

Actually, it seems that eg. recover_output() does both fseeko()
and lseek(fileno(fp)) on the same file (the latter through
myseeko()). Is this actually so? And if yes, what does it exactly
mean? Can we substitute _lseeki64(fileno(fp), offset, origin) for
fseeko(fd, offset, whence) when _FILE_OFFSET_BITS is set to 64 or
do the two calls have different semantics?


I also updated some of the SConscript files in CVS, most
prominently to work with the new tifflib, and a few other more
cosmetic changes.


-schorsch

-- 
Georg Mischler  --  simulations developer  --  schorsch at schorsch com
+schorsch.com+  --  lighting design tools  --  http://www.schorsch.com/