Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

 Angus == Angus Leeming [EMAIL PROTECTED] writes:

Angus You're just too clever for me ;-) However, static member
Angus functions can't inherit

Why is that?

Angus If you want to do something like that, then some sort of
Angus indirection with class os hiding a member variable, is
Angus inevitable I fear.

Angus Anyway, the principle point remains. Do you want to do this in
Angus the 1.3.x series or should we leave that as ugly but working?

OK, forget about that for the moment. It was just an idea that is not
needed now.

JMarc

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
 Angus You're just too clever for me ;-) However, static member
 Angus functions can't inherit
 
 Why is that?

That's just the way the language works.

 Angus If you want to do something like that, then some sort of
 Angus indirection with class os hiding a member variable, is
 Angus inevitable I fear.
 
 Angus Anyway, the principle point remains. Do you want to do this in
 Angus the 1.3.x series or should we leave that as ugly but working?
 
 OK, forget about that for the moment. It was just an idea that is not
 needed now.

What I do intend to do is follow up your idea of creating os_cygwin.C. 
First, however, let's get the functionality into the existing os_win32.C. 
It'll help us see what the differences between cygwin and windows are.

-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes:

Angus> You're just too clever for me ;-) However, static member
Angus> functions can't inherit

Why is that?

Angus> If you want to do something like that, then some sort of
Angus> indirection with "class os" hiding a member variable, is
Angus> inevitable I fear.

Angus> Anyway, the principle point remains. Do you want to do this in
Angus> the 1.3.x series or should we leave that as "ugly but working"?

OK, forget about that for the moment. It was just an idea that is not
needed now.

JMarc

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
> Angus> You're just too clever for me ;-) However, static member
> Angus> functions can't inherit
> 
> Why is that?

That's just the way the language works.

> Angus> If you want to do something like that, then some sort of
> Angus> indirection with "class os" hiding a member variable, is
> Angus> inevitable I fear.
> 
> Angus> Anyway, the principle point remains. Do you want to do this in
> Angus> the 1.3.x series or should we leave that as "ugly but working"?
> 
> OK, forget about that for the moment. It was just an idea that is not
> needed now.

What I do intend to do is follow up your idea of creating os_cygwin.C. 
First, however, let's get the functionality into the existing os_win32.C. 
It'll help us see what the differences between cygwin and windows are.

-- 
Angus

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| As promised, I'm posting the remainder of the changes that are needed to
| compile LyX with MinGW. This patch is much less intrusive than the
| original because the offensive os_win32.h is how #included only by
| those .C files that actually need it.

I am not sure I like that solution.

We should work hard to get all platform specific code out of the
regular .C files.

If a support/os.h works equally well we should use that instead.
(and no conditionals)

| Index: src/insets/insetgraphics.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetgraphics.C,v
| retrieving revision 1.146.2.4
| diff -u -p -r1.146.2.4 insetgraphics.C
| --- src/insets/insetgraphics.C7 Dec 2004 10:49:34 -   
1.146.2.4
| +++ src/insets/insetgraphics.C16 Dec 2004 01:04:46 -
| @@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
|   // without dots and again with ext
|   temp_file = ChangeExtension(
|   subst(temp_file, ., _), ext_tmp);
| + //Remove drive letter on Win32
| + if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);

split on two lines

|   // now we have any_dir_file.ext
|   temp_file = MakeAbsPath(temp_file, buf-tmppath);
|   lyxerr[Debug::GRAPHICS]
| Index: src/support/FileInfo.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/FileInfo.C,v
| retrieving revision 1.18.2.2
| diff -u -p -r1.18.2.2 FileInfo.C
| --- src/support/FileInfo.C15 Dec 2004 21:40:03 -  1.18.2.2
| +++ src/support/FileInfo.C16 Dec 2004 01:04:46 -
| @@ -174,10 +178,20 @@ void FileInfo::init()
|  
|  void FileInfo::dostat(bool link)
|  {
| + string name(fname_);
| +#ifdef _WIN32
| + // Win32 stat() doesn't dig trailing slashes
| + if (name.at(name.size()-1) == '/') name.erase(name.size() -1);

ditto 

| Index: src/support/filetools.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/filetools.C,v
| retrieving revision 1.146.2.7
| diff -u -p -r1.146.2.7 filetools.C
| --- src/support/filetools.C   15 Dec 2004 19:35:11 -  1.146.2.7
| +++ src/support/filetools.C   16 Dec 2004 01:04:48 -
| @@ -193,10 +196,12 @@ string const FileOpenSearch(string const
|   notfound = false;
|   }
|   }
| -#ifdef __EMX__
| +#if defined(__EMX__) || defined(_WIN32) 
|   if (ext.empty()  notfound) {
|   real_file = FileOpenSearch(path, name, exe);
| - if (notfound) real_file = FileOpenSearch(path, name, cmd);
| +#ifdef __EMX__
| + if (notfound) real_file = FileOpenSearch(path, name,
| cmd);

ditto (even if not done before)

| Index: src/support/kill.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/kill.C,v
| retrieving revision 1.7
| diff -u -p -r1.7 kill.C
| --- src/support/kill.C10 Jun 2002 17:31:57 -  1.7
| +++ src/support/kill.C16 Dec 2004 01:04:48 -
| @@ -5,7 +5,40 @@
|  #include sys/types.h
|  #include csignal
|  
| +#ifdef _WIN32
| +#include debug.h
| +#include os.h
| +
| +#include windows.h
| +#include errno.h

 cerrno ??

| +
| +using std::endl;
| +#endif //_WIN32

and debug.h, os.h, std::endl is not specific enough for WIN32 to be
inside a ifdef, just put them outside.

| +
|  int lyx::kill(int pid, int sig)
|  {
| +#ifdef _WIN32
| + if (pid == (int)GetCurrentProcessId())
| + return -(raise(sig));

Hmm.. Do we ever send signals to ourselves. Perhaps we should just
assert on that condition.

Either that or handle win/non-win in the same way.

| + else{
| + HANDLE hProcess;
| + if (!(hProcess =
| + OpenProcess(PROCESS_ALL_ACCESS, TRUE, pid))) {
| + lyxerr  kill OpenProcess failed!  endl;
| + return -1;
| + }
| + else {
| + if (!TerminateProcess(hProcess, sig)){
| + lyxerr  kill process failed!  endl;
| + CloseHandle(hProcess);
| + return -1;
| + }
| + CloseHandle(hProcess);
| + }
| + }
| + return 0;
| +
| +#else
|   return ::kill(pid, sig);

And we should probably check this for errors as well.
(and log them)

-- 
Lgb

Re: The remainder of Ruurd's changes

[Angus Leeming]

Lars Gullik Bjønnes wrote:
 | As promised, I'm posting the remainder of the changes that are needed
 | to compile LyX with MinGW. This patch is much less intrusive than the
 | original because the offensive os_win32.h is how #included only by
 | those .C files that actually need it.
 
 I am not sure I like that solution.
 
 We should work hard to get all platform specific code out of the
 regular .C files.
 
 If a support/os.h works equally well we should use that instead.
 (and no conditionals)

You misunderstand me. (I was unable to express myself clearly at one 
o'clock in the morning.)

I am not proposing that this patch should go into the tree. Anything but. 
I am saying that this is the patch that is needed to make LyX compile on 
Windows. Things in the patch can be classified in three groups:

1. Things to do with using LyX on a Windows machine. That is, the stuff in 
os_win32.C (The .C file, not the .h file which is a kludge). The 
insetgraphics.C fix. These things will probably go into the tree pretty 
much as-is (once I actually understand what they do ;-))

2. Things to do with files. For example, symbolic links do not exist on 
Windows. There are changes to filetools.C and FileInfo.C that will be 
needed but which are masked by the empty macros in os_win32.h. These 
things will need some configure-magic but the basic code is probably fine.

3. Things to do with external processes. Launching, 'em, communicating 
with 'em throught FIFOs and killing 'em. ispell.C, lyxserver.C, 
lyx_cb.C, kill.C, forkedcall.C, forkedcontrl.C. The empty macros of 
os_win32.h hide the fact that the Windows API is fundamentally different 
to the Posix one. This code will need to be re-written entirely in order 
to provide Windows users with the same power as *nix ones.

Addressing the issues in the first two of these groups is straightforward. 
Group 3 will need more work and the changes may well be unsuited to the 
1.3.x branch. Time will tell.

-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

 Angus == Angus Leeming [EMAIL PROTECTED] writes:

Angus As promised, I'm posting the remainder of the changes that are
Angus needed to compile LyX with MinGW. This patch is much less
Angus intrusive than the original because the offensive os_win32.h is
Angus how #included only by those .C files that actually need it.

   +#ifdef _WIN32
   +# include support/os_win32.h
   +#endif
   +

I am not very fond of this thing. Could you at least in each of these
instance add a comment telling what construct is missing? Then we
could try to have a strategy on avoiding some of this stuff. 

The same holds for the other instances.
 
   +#ifdef HAVE_UNISTD_H
   +# include unistd.h
   +#endif
   +

cunistd?

 
   +#ifdef HAVE_SELECT
   retval = ::select(SELECT_TYPE_ARG1 (max(pipeout[0], pipeerr[0]) + 1),
   SELECT_TYPE_ARG234 (infds),
   0,
   0,
   SELECT_TYPE_ARG5 (tv));
   +#else
   +retval = -1;
   +#endif

Does it mean that ispell will not work in windows?

   @@ -289,7 +293,7 @@ void LyX::init(bool gui)
   do {
   // Path of binary/../share/name of binary/
   searchpath += NormalizePath(AddPath(binpath, ../share/) +
   -  OnlyFilename(binname)) + ';';
   +  ChangeExtension(OnlyFilename(binname),)) + ';';

Don't we need something more selective, like removing .exe from the
end of the string if it is here? Actually, I guess the whole LyX::init
strategy should be rethought in the light of our platform support.

A little digression: instead of our current os:: namespace, wouldn't
it be better to have a hierarchy of os_foo classes with static
methods, so that some environment could inherit others (like cygwin =
unix + some stuff). The some of the code from this init method could
be moved to os_foo.C files.


   @@ -113,6 +113,7 @@ string const fromqstr(QString const  st
{
   QTextCodec * codec = QTextCodec::codecForLocale();
   QCString tmpstr = codec-fromUnicode(str);
   -char const * tmpcstr = tmpstr;
   +char const * tmpcstr = \0;
   +if (!tmpstr.isEmpty()) tmpcstr = tmpstr;
   return tmpcstr;
}

Do you know what this does exactly? The indentation is wrong anyway.

   @@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
   // without dots and again with ext
   temp_file = ChangeExtension(
   subst(temp_file, ., _), ext_tmp);
   +//Remove drive letter on Win32
   +if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);
   // now we have any_dir_file.ext
   temp_file = MakeAbsPath(temp_file, buf-tmppath);
   lyxerr[Debug::GRAPHICS]

I do not like this. Is there a reason why this should not be handled
by some generic function?

   @@ -193,10 +196,12 @@ string const FileOpenSearch(string const
   notfound = false;
   }
   }
   -#ifdef __EMX__
   +#if defined(__EMX__) || defined(_WIN32) 
   if (ext.empty()  notfound) {
   real_file = FileOpenSearch(path, name, exe);
   -if (notfound) real_file = FileOpenSearch(path, name, cmd);
   +#ifdef __EMX__
   +if (notfound) real_file = FileOpenSearch(path, name, cmd);
   +#endif

If OS/2 looks for .cmd, should win32 look for .bat? 

The test should be on two lines.

   @@ -366,7 +371,7 @@ string const GetEnv(string const  envna

string const GetEnvPath(string const  name)
{
   -#ifndef __EMX__
   +#if !defined(__EMX__)  !defined(_WIN32)
   string const pathlist = subst(GetEnv(name), ':', ';');
#else
   string const pathlist = os::slashify_path(GetEnv(name));

First, there are a lot of lines like
  src/lyx_main.C:using lyx::support::GetEnvPath;
that should be removed from the source

Then, when you look at the remaining uses of GetEnvPath (in HEAD), you
find two categories:

- stuff in os_foo.C, which can have its own code without ifdef

- stuff in path_defines.C.in, which does not consider list of paths
  anyway

So I think we could maybe get rid of the function. Also we could
decide to change os::slashify_path so that it does the :-;
substitution as needed.

JMarc

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Jean-Marc Lasgouttes [EMAIL PROTECTED] writes:

|+#ifdef HAVE_UNISTD_H
|+# include unistd.h
|+#endif
|+

| cunistd?

No such thing.

-- 
Lgb

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Jean-Marc Lasgouttes [EMAIL PROTECTED] writes:

 Lars == Lars Gullik Bjønnes [EMAIL PROTECTED] writes:

| | cunistd?

| Lars No such thing.

| What can unistd.h do that no c* header does?

unixy things. Lots of stuff just have a look at the header on your own
box.

The c* header are basically just the C-standard headers in C++
wrapping.

-- 
Lgb

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
 A little digression: instead of our current os:: namespace, wouldn't
 it be better to have a hierarchy of os_foo classes with static
 methods, so that some environment could inherit others (like cygwin =
 unix + some stuff). The some of the code from this init method could
 be moved to os_foo.C files.

You can't have static virtual member functions, but you could have either 
a class os (à la 13x) or a namespace os (à la 14x) that hid the 
implementation:

class os {
public:
static void init(int argc, char * argv[]);

static string const  binpath();
static string const  binname();
static string const  homepath();
static string const  nulldev();
private:
struct impl {
virtual string const  binpath() const = 0;
virtual string const  binname() const = 0;
virtual string const  homepath() const = 0;
virtual string const  nulldev() const = 0;
};
static boost::scoped_ptrimpl pimpl_;
};

Each os_cygwin, os_os2, os_unix, os_win32 class would inherit from 
os::impl.

However, I don't think that such a change should go in the 1.3.x series 
and I don't really see what it would give 1.4.x. Can we leave this for 
1.5.x?

-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

 Angus == Angus Leeming [EMAIL PROTECTED] writes:

Angus Jean-Marc Lasgouttes wrote:
 A little digression: instead of our current os:: namespace,
 wouldn't it be better to have a hierarchy of os_foo classes with
 static methods, so that some environment could inherit others (like
 cygwin = unix + some stuff). The some of the code from this init
 method could be moved to os_foo.C files.

Angus You can't have static virtual member functions, but you could
Angus have either a class os (à la 13x) or a namespace os (à la
Angus 14x) that hid the implementation:

Did I say virtual? I thought of static member functions and os.h would
do

#if in unix 
#include os_unix.h
typedef os_unix os;
#else if in win32
#include os_win32.h
typedef os_win32 os;
...

JMarc

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:

@@ -289,7 +293,7 @@ void LyX::init(bool gui)
do {
// Path of binary/../share/name of binary/
searchpath += NormalizePath(AddPath(binpath,
../share/) +
-OnlyFilename(binname)) + ';';
+ChangeExtension(OnlyFilename(binname),)) + ';';
 
 Don't we need something more selective, like removing .exe from the
 end of the string if it is here? Actually, I guess the whole LyX::init
 strategy should be rethought in the light of our platform support.

This is an equivalent can of worms to the whole --with-version-suffix 
discussion isn't it? One plausible suggestion (made by Lars?) is to 
hard-code the location of the system-wide shared data as
/Path of binary/../share/lyx/13x/

where 13x would actually have been 130 for LyX 1.3.0 to 1.3.3 and would 
have changed to 134 for LyX version 1.3.4 when a change to the text class 
format was introduced. Maybe that's a bit too cute and each separate LyX 
version should have its own directory.

Ditto, the userdir would become:
os::homepath()/.lyx/13x/

It seems to me that such a scheme would work on all platforms. Am I 
missing something?

Returning to the particular point raised by the proposed patch. Would you 
be happy if I explicitly stripped .exe for the Win32 case? Something 
like:

do {
// Path of binary/../share/name of binary/
string const exe_name = OnlyFilename(binname);
string const lyx_system_dir_name =
#ifdef _WIN32
suffixIs(exe_name, .exe) ?
ChangeExtension(exe_name, ) :
exe_name;
#else
exe_name;
#endif
string const shared_dir_name =
NormalizePath(AddPath(binpath, ../share/);
searchpath += shared_dir_name + lyx_system_dir_name + ';';

Or do you want to propogate an entire new scheme back into the 13x series?

-- 
Angus

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
@@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
// without dots and again with ext
temp_file = ChangeExtension(
subst(temp_file, ., _), ext_tmp);
+  //Remove drive letter on Win32
+  if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);
// now we have any_dir_file.ext
temp_file = MakeAbsPath(temp_file, buf-tmppath);
lyxerr[Debug::GRAPHICS]
 
 I do not like this. Is there a reason why this should not be handled
 by some generic function?

It is in 1.4.x. The file name is stored as a FileName variable and this 
mangling is done via FileName::mangledFilename()

Anyway, I think that the proposal should be:

+  // Mangle drive name on Win32
+  if (temp_file[1] == ':') temp_file[1] = '_';

Ie, the temporary file's name should include this information about the 
drive.

I'll prepare patches.

-- 
Angus

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
 A little digression: instead of our current os:: namespace,
 wouldn't it be better to have a hierarchy of os_foo classes with
 static methods, so that some environment could inherit others (like
 cygwin = unix + some stuff). The some of the code from this init
 method could be moved to os_foo.C files.
 
 Angus You can't have static virtual member functions, but you could
 Angus have either a class os (à la 13x) or a namespace os (à la
 Angus 14x) that hid the implementation:
 
 Did I say virtual? I thought of static member functions and os.h would
 do
 
 #if in unix
 #include os_unix.h
 typedef os_unix os;
 #else if in win32
 #include os_win32.h
 typedef os_win32 os;
 ...

You're just too clever for me ;-) However, static member functions can't 
inherit, which is what I thought the point was:

class os_unix {};
class os_cygwin : public os_unix {
// Override some specific member function.
};

If you want to do something like that, then some sort of indirection with 
class os hiding a member variable, is inevitable I fear.

Anyway, the principle point remains. Do you want to do this in the 1.3.x 
series or should we leave that as ugly but working?
-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

 Angus == Angus Leeming [EMAIL PROTECTED] writes:

Angus Thanks for the commentary, Jean-Marc. I like the ideas of a
Angus hierarchy of os classes and of getting rid of GetEnvPath.
Angus I'll continue the process of small, self contained patches.

Sure.

Angus I think that, as a rule, we should be a little less worried
Angus about indentation issues. As often as not the only comments
Angus that a patch gets are about indentation and whitespace. Let's
Angus assume that I'll get that right and concentrate on the
Angus substance of the proposed changes ;-)

Agreed.

Angus Summary, the above code is a bug fix, pure and simple, although
Angus I'd write is as:

Angus { QTextCodec const * const codec =
Angus QTextCodec::codecForLocale(); QCString const tmpstr =
Angus codec-fromUnicode(str); return tmpstr.isEmpty() ? string() :
Angus string(tmpstr);
Angus }

Angus Patches attached. Happy if I commit?

Yes. I was wondering whether this is related to the string-related
crashes people have been experiencing with some versions of Qt (with
non-existing fonts). I believe this has been fixed in 1.4.0cvs, but
not in 1.3.6cvs.

JMarc

Re: The remainder of Ruurd's changes

[Angus Leeming]

Lars Gullik Bjønnes wrote:
 | class os {
 | public:
 | static void init(int argc, char * argv[]);

 | static string const  binpath();
 | static string const  binname();
 | static string const  homepath();
 | static string const  nulldev();
 | private:
 | struct impl {
 | virtual string const  binpath() const = 0;
 | virtual string const  binname() const = 0;
 | virtual string const  homepath() const = 0;
 | virtual string const  nulldev() const = 0;
 | };
 | static boost::scoped_ptrimpl pimpl_;
 | };
 
 Will this work? Remember that you can call a static function without
 the object.
 
 (Ok I see what you want to do. (init should be private probably))

No, os::init() is called from main(int argc, char * argv).

A lot of these 'theoretical' problems with global data and order of 
initialization would go away if we had a single global LyX variable that 
held all other, currently global, variables.

Anyway, I class this all as a 'make the code nicer' project with no real 
user gains. I don't really see what it has to do with trying to get LyX 
running on Windows. In fact, producing something that works but is even 
clunkier than the existing code base (more #ifdefs) might provide even 
more motivation to refactor this piece of the code ;-)

Whereever this conversation goes, I don't think that any refactoring 
should be backported into the 1.3.x tree.

-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

 Lars == Lars Gullik Bjønnes [EMAIL PROTECTED] writes:

| cunistd?

Lars No such thing.

What can unistd.h do that no c* header does?

JMarc

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| Jean-Marc Lasgouttes wrote:
 A little digression: instead of our current os:: namespace, wouldn't
 it be better to have a hierarchy of os_foo classes with static
 methods, so that some environment could inherit others (like cygwin =
 unix + some stuff). The some of the code from this init method could
 be moved to os_foo.C files.

| You can't have static virtual member functions, but you could have either 
| a class os (à la 13x) or a namespace os (à la 14x) that hid the 
| implementation:

| class os {
| public:
| static void init(int argc, char * argv[]);

| static string const  binpath();
| static string const  binname();
| static string const  homepath();
| static string const  nulldev();
| private:
| struct impl {
| virtual string const  binpath() const = 0;
| virtual string const  binname() const = 0;
| virtual string const  homepath() const = 0;
| virtual string const  nulldev() const = 0;
| };
| static boost::scoped_ptrimpl pimpl_;
| };

Will this work? Remember that you can call a static function without
the object.

(Ok I see what you want to do. (init should be private probably))

-- 
Lgb

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| As promised, I'm posting the remainder of the changes that are needed to
| compile LyX with MinGW. This patch is much less intrusive than the
| original because the offensive os_win32.h is how #included only by
| those .C files that actually need it.

I am not sure I like that solution.

We should work hard to get all platform specific code out of the
regular .C files.

If a support/os.h works equally well we should use that instead.
(and no conditionals)

| Index: src/insets/insetgraphics.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetgraphics.C,v
| retrieving revision 1.146.2.4
| diff -u -p -r1.146.2.4 insetgraphics.C
| --- src/insets/insetgraphics.C7 Dec 2004 10:49:34 -   
1.146.2.4
| +++ src/insets/insetgraphics.C16 Dec 2004 01:04:46 -
| @@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
|   // without dots and again with ext
|   temp_file = ChangeExtension(
|   subst(temp_file, ".", "_"), ext_tmp);
| + //Remove drive letter on Win32
| + if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);

split on two lines

|   // now we have any_dir_file.ext
|   temp_file = MakeAbsPath(temp_file, buf->tmppath);
|   lyxerr[Debug::GRAPHICS]
| Index: src/support/FileInfo.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/FileInfo.C,v
| retrieving revision 1.18.2.2
| diff -u -p -r1.18.2.2 FileInfo.C
| --- src/support/FileInfo.C15 Dec 2004 21:40:03 -  1.18.2.2
| +++ src/support/FileInfo.C16 Dec 2004 01:04:46 -
| @@ -174,10 +178,20 @@ void FileInfo::init()
|  
|  void FileInfo::dostat(bool link)
|  {
| + string name(fname_);
| +#ifdef _WIN32
| + // Win32 stat() doesn't dig trailing slashes
| + if (name.at(name.size()-1) == '/') name.erase(name.size() -1);

ditto 

| Index: src/support/filetools.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/filetools.C,v
| retrieving revision 1.146.2.7
| diff -u -p -r1.146.2.7 filetools.C
| --- src/support/filetools.C   15 Dec 2004 19:35:11 -  1.146.2.7
| +++ src/support/filetools.C   16 Dec 2004 01:04:48 -
| @@ -193,10 +196,12 @@ string const FileOpenSearch(string const
|   notfound = false;
|   }
|   }
| -#ifdef __EMX__
| +#if defined(__EMX__) || defined(_WIN32) 
|   if (ext.empty() && notfound) {
|   real_file = FileOpenSearch(path, name, "exe");
| - if (notfound) real_file = FileOpenSearch(path, name, "cmd");
| +#ifdef __EMX__
| + if (notfound) real_file = FileOpenSearch(path, name,
| "cmd");

ditto (even if not done before)

| Index: src/support/kill.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/kill.C,v
| retrieving revision 1.7
| diff -u -p -r1.7 kill.C
| --- src/support/kill.C10 Jun 2002 17:31:57 -  1.7
| +++ src/support/kill.C16 Dec 2004 01:04:48 -
| @@ -5,7 +5,40 @@
|  #include 
|  #include 
|  
| +#ifdef _WIN32
| +#include "debug.h"
| +#include "os.h"
| +
| +#include 
| +#include 

  ??

| +
| +using std::endl;
| +#endif //_WIN32

and debug.h, os.h, std::endl is not specific enough for WIN32 to be
inside a ifdef, just put them outside.

| +
|  int lyx::kill(int pid, int sig)
|  {
| +#ifdef _WIN32
| + if (pid == (int)GetCurrentProcessId())
| + return -(raise(sig));

Hmm.. Do we ever send signals to ourselves. Perhaps we should just
assert on that condition.

Either that or handle win/non-win in the same way.

| + else{
| + HANDLE hProcess;
| + if (!(hProcess =
| + OpenProcess(PROCESS_ALL_ACCESS, TRUE, pid))) {
| + lyxerr << "kill OpenProcess failed!" << endl;
| + return -1;
| + }
| + else {
| + if (!TerminateProcess(hProcess, sig)){
| + lyxerr << "kill process failed!" << endl;
| + CloseHandle(hProcess);
| + return -1;
| + }
| + CloseHandle(hProcess);
| + }
| + }
| + return 0;
| +
| +#else
|   return ::kill(pid, sig);

And we should probably check this for errors as well.
(and log them)

-- 
Lgb

Re: The remainder of Ruurd's changes

[Angus Leeming]

Lars Gullik Bjønnes wrote:
> | As promised, I'm posting the remainder of the changes that are needed
> | to compile LyX with MinGW. This patch is much less intrusive than the
> | original because the offensive os_win32.h is how #included only by
> | those .C files that actually need it.
> 
> I am not sure I like that solution.
> 
> We should work hard to get all platform specific code out of the
> regular .C files.
> 
> If a support/os.h works equally well we should use that instead.
> (and no conditionals)

You misunderstand me. (I was unable to express myself clearly at one 
o'clock in the morning.)

I am not proposing that this patch should go into the tree. Anything but. 
I am saying that this is the patch that is needed to make LyX compile on 
Windows. Things in the patch can be classified in three groups:

1. Things to do with using LyX on a Windows machine. That is, the stuff in 
os_win32.C (The .C file, not the .h file which is a kludge). The 
insetgraphics.C fix. These things will probably go into the tree pretty 
much as-is (once I actually understand what they do ;-))

2. Things to do with files. For example, symbolic links do not exist on 
Windows. There are changes to filetools.C and FileInfo.C that will be 
needed but which are masked by the empty macros in os_win32.h. These 
things will need some configure-magic but the basic code is probably fine.

3. Things to do with external processes. Launching, 'em, communicating 
with 'em throught FIFOs and killing 'em. ispell.C, lyxserver.C, 
lyx_cb.C, kill.C, forkedcall.C, forkedcontrl.C. The empty macros of 
os_win32.h hide the fact that the Windows API is fundamentally different 
to the Posix one. This code will need to be re-written entirely in order 
to provide Windows users with the same power as *nix ones.

Addressing the issues in the first two of these groups is straightforward. 
Group 3 will need more work and the changes may well be unsuited to the 
1.3.x branch. Time will tell.

-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes:

Angus> As promised, I'm posting the remainder of the changes that are
Angus> needed to compile LyX with MinGW. This patch is much less
Angus> intrusive than the original because the offensive os_win32.h is
Angus> how #included only by those .C files that actually need it.

   +#ifdef _WIN32
   +# include "support/os_win32.h"
   +#endif
   +

I am not very fond of this thing. Could you at least in each of these
instance add a comment telling what construct is missing? Then we
could try to have a strategy on avoiding some of this stuff. 

The same holds for the other instances.
 
   +#ifdef HAVE_UNISTD_H
   +# include 
   +#endif
   +

?

 
   +#ifdef HAVE_SELECT
   retval = ::select(SELECT_TYPE_ARG1 (max(pipeout[0], pipeerr[0]) + 1),
   SELECT_TYPE_ARG234 (),
   0,
   0,
   SELECT_TYPE_ARG5 ());
   +#else
   +retval = -1;
   +#endif

Does it mean that ispell will not work in windows?

   @@ -289,7 +293,7 @@ void LyX::init(bool gui)
   do {
   // Path of binary/../share/name of binary/
   searchpath += NormalizePath(AddPath(binpath, "../share/") +
   -  OnlyFilename(binname)) + ';';
   +  ChangeExtension(OnlyFilename(binname),"")) + ';';

Don't we need something more selective, like removing ".exe" from the
end of the string if it is here? Actually, I guess the whole LyX::init
strategy should be rethought in the light of our platform support.

A little digression: instead of our current os:: namespace, wouldn't
it be better to have a hierarchy of os_foo classes with static
methods, so that some environment could inherit others (like cygwin =
unix + some stuff). The some of the code from this init method could
be moved to os_foo.C files.


   @@ -113,6 +113,7 @@ string const fromqstr(QString const & st
{
   QTextCodec * codec = QTextCodec::codecForLocale();
   QCString tmpstr = codec->fromUnicode(str);
   -char const * tmpcstr = tmpstr;
   +char const * tmpcstr = "\0";
   +if (!tmpstr.isEmpty()) tmpcstr = tmpstr;
   return tmpcstr;
}

Do you know what this does exactly? The indentation is wrong anyway.

   @@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
   // without dots and again with ext
   temp_file = ChangeExtension(
   subst(temp_file, ".", "_"), ext_tmp);
   +//Remove drive letter on Win32
   +if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);
   // now we have any_dir_file.ext
   temp_file = MakeAbsPath(temp_file, buf->tmppath);
   lyxerr[Debug::GRAPHICS]

I do not like this. Is there a reason why this should not be handled
by some generic function?

   @@ -193,10 +196,12 @@ string const FileOpenSearch(string const
   notfound = false;
   }
   }
   -#ifdef __EMX__
   +#if defined(__EMX__) || defined(_WIN32) 
   if (ext.empty() && notfound) {
   real_file = FileOpenSearch(path, name, "exe");
   -if (notfound) real_file = FileOpenSearch(path, name, "cmd");
   +#ifdef __EMX__
   +if (notfound) real_file = FileOpenSearch(path, name, "cmd");
   +#endif

If OS/2 looks for .cmd, should win32 look for .bat? 

The test should be on two lines.

   @@ -366,7 +371,7 @@ string const GetEnv(string const & envna

string const GetEnvPath(string const & name)
{
   -#ifndef __EMX__
   +#if !defined(__EMX__) && !defined(_WIN32)
   string const pathlist = subst(GetEnv(name), ':', ';');
#else
   string const pathlist = os::slashify_path(GetEnv(name));

First, there are a lot of lines like
  src/lyx_main.C:using lyx::support::GetEnvPath;
that should be removed from the source

Then, when you look at the remaining uses of GetEnvPath (in HEAD), you
find two categories:

- stuff in os_foo.C, which can have its own code without ifdef

- stuff in path_defines.C.in, which does not consider list of paths
  anyway

So I think we could maybe get rid of the function. Also we could
decide to change os::slashify_path so that it does the :->;
substitution as needed.

JMarc

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes:

|+#ifdef HAVE_UNISTD_H
|+# include 
|+#endif
|+

| ?

No such thing.

-- 
Lgb

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes:

>> "Lars" == Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:
>
| | ?
>
| Lars> No such thing.
>
| What can unistd.h do that no  header does?

unixy things. Lots of stuff just have a look at the header on your own
box.

The  header are basically just the C-standard headers in C++
wrapping.

-- 
Lgb

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
> A little digression: instead of our current os:: namespace, wouldn't
> it be better to have a hierarchy of os_foo classes with static
> methods, so that some environment could inherit others (like cygwin =
> unix + some stuff). The some of the code from this init method could
> be moved to os_foo.C files.

You can't have static virtual member functions, but you could have either 
a "class os" (à la 13x) or a "namespace os" (à la 14x) that hid the 
implementation:

class os {
public:
static void init(int argc, char * argv[]);

static string const & binpath();
static string const & binname();
static string const & homepath();
static string const & nulldev();
private:
struct impl {
virtual string const & binpath() const = 0;
virtual string const & binname() const = 0;
virtual string const & homepath() const = 0;
virtual string const & nulldev() const = 0;
};
static boost::scoped_ptr pimpl_;
};

Each os_cygwin, os_os2, os_unix, os_win32 class would inherit from 
os::impl.

However, I don't think that such a change should go in the 1.3.x series 
and I don't really see what it would give 1.4.x. Can we leave this for 
1.5.x?

-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes:

Angus> Jean-Marc Lasgouttes wrote:
>> A little digression: instead of our current os:: namespace,
>> wouldn't it be better to have a hierarchy of os_foo classes with
>> static methods, so that some environment could inherit others (like
>> cygwin = unix + some stuff). The some of the code from this init
>> method could be moved to os_foo.C files.

Angus> You can't have static virtual member functions, but you could
Angus> have either a "class os" (à la 13x) or a "namespace os" (à la
Angus> 14x) that hid the implementation:

Did I say virtual? I thought of static member functions and os.h would
do

#if in unix 
#include "os_unix.h"
typedef os_unix os;
#else if in win32
#include "os_win32.h"
typedef os_win32 os;
...

JMarc

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:

>@@ -289,7 +293,7 @@ void LyX::init(bool gui)
>do {
>// Path of binary/../share/name of binary/
>searchpath += NormalizePath(AddPath(binpath,
>"../share/") +
>-OnlyFilename(binname)) + ';';
>+ChangeExtension(OnlyFilename(binname),"")) + ';';
> 
> Don't we need something more selective, like removing ".exe" from the
> end of the string if it is here? Actually, I guess the whole LyX::init
> strategy should be rethought in the light of our platform support.

This is an equivalent can of worms to the whole "--with-version-suffix" 
discussion isn't it? One plausible suggestion (made by Lars?) is to 
hard-code the location of the system-wide shared data as
/Path of binary/../share/lyx/13x/

where 13x would actually have been 130 for LyX 1.3.0 to 1.3.3 and would 
have changed to 134 for LyX version 1.3.4 when a change to the text class 
format was introduced. Maybe that's a bit too cute and each separate LyX 
version should have its own directory.

Ditto, the userdir would become:
os::homepath()/.lyx/13x/

It seems to me that such a scheme would work on all platforms. Am I 
missing something?

Returning to the particular point raised by the proposed patch. Would you 
be happy if I explicitly stripped ".exe" for the Win32 case? Something 
like:

do {
// Path of binary/../share/name of binary/
string const exe_name = OnlyFilename(binname);
string const lyx_system_dir_name =
#ifdef _WIN32
suffixIs(exe_name, ".exe") ?
ChangeExtension(exe_name, "") :
exe_name;
#else
exe_name;
#endif
string const shared_dir_name =
NormalizePath(AddPath(binpath, "../share/");
searchpath += shared_dir_name + lyx_system_dir_name + ';';

Or do you want to propogate an entire new scheme back into the 13x series?

-- 
Angus

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
>@@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
>// without dots and again with ext
>temp_file = ChangeExtension(
>subst(temp_file, ".", "_"), ext_tmp);
>+  //Remove drive letter on Win32
>+  if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);
>// now we have any_dir_file.ext
>temp_file = MakeAbsPath(temp_file, buf->tmppath);
>lyxerr[Debug::GRAPHICS]
> 
> I do not like this. Is there a reason why this should not be handled
> by some generic function?

It is in 1.4.x. The file name is stored as a FileName variable and this 
mangling is done via FileName::mangledFilename()

Anyway, I think that the proposal should be:

>+  // Mangle drive name on Win32
>+  if (temp_file[1] == ':') temp_file[1] = '_';

Ie, the temporary file's name should include this information about the 
drive.

I'll prepare patches.

-- 
Angus

Re: The remainder of Ruurd's changes

[Angus Leeming]

Jean-Marc Lasgouttes wrote:
>>> A little digression: instead of our current os:: namespace,
>>> wouldn't it be better to have a hierarchy of os_foo classes with
>>> static methods, so that some environment could inherit others (like
>>> cygwin = unix + some stuff). The some of the code from this init
>>> method could be moved to os_foo.C files.
> 
> Angus> You can't have static virtual member functions, but you could
> Angus> have either a "class os" (à la 13x) or a "namespace os" (à la
> Angus> 14x) that hid the implementation:
> 
> Did I say virtual? I thought of static member functions and os.h would
> do
> 
> #if in unix
> #include "os_unix.h"
> typedef os_unix os;
> #else if in win32
> #include "os_win32.h"
> typedef os_win32 os;
> ...

You're just too clever for me ;-) However, static member functions can't 
inherit, which is what I thought the point was:

class os_unix {};
class os_cygwin : public os_unix {
// Override some specific member function.
};

If you want to do something like that, then some sort of indirection with 
"class os" hiding a member variable, is inevitable I fear.

Anyway, the principle point remains. Do you want to do this in the 1.3.x 
series or should we leave that as "ugly but working"?
-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes:

Angus> Thanks for the commentary, Jean-Marc. I like the ideas of a
Angus> hierarchy of "os" classes and of getting rid of GetEnvPath.
Angus> I'll continue the process of small, self contained patches.

Sure.

Angus> I think that, as a rule, we should be a little less worried
Angus> about indentation issues. As often as not the only comments
Angus> that a patch gets are about indentation and whitespace. Let's
Angus> assume that I'll get that right and concentrate on the
Angus> substance of the proposed changes ;-)

Agreed.

Angus> Summary, the above code is a bug fix, pure and simple, although
Angus> I'd write is as:

Angus> { QTextCodec const * const codec =
Angus> QTextCodec::codecForLocale(); QCString const tmpstr =
Angus> codec->fromUnicode(str); return tmpstr.isEmpty() ? string() :
Angus> string(tmpstr);
Angus> }

Angus> Patches attached. Happy if I commit?

Yes. I was wondering whether this is related to the string-related
crashes people have been experiencing with some versions of Qt (with
non-existing fonts). I believe this has been fixed in 1.4.0cvs, but
not in 1.3.6cvs.

JMarc

Re: The remainder of Ruurd's changes

[Angus Leeming]

Lars Gullik Bjønnes wrote:
> | class os {
> | public:
> | static void init(int argc, char * argv[]);
>>
> | static string const & binpath();
> | static string const & binname();
> | static string const & homepath();
> | static string const & nulldev();
> | private:
> | struct impl {
> | virtual string const & binpath() const = 0;
> | virtual string const & binname() const = 0;
> | virtual string const & homepath() const = 0;
> | virtual string const & nulldev() const = 0;
> | };
> | static boost::scoped_ptr pimpl_;
> | };
> 
> Will this work? Remember that you can call a static function without
> the object.
> 
> (Ok I see what you want to do. (init should be private probably))

No, os::init() is called from main(int argc, char * argv).

A lot of these 'theoretical' problems with global data and order of 
initialization would go away if we had a single global LyX variable that 
held all other, currently global, variables.

Anyway, I class this all as a 'make the code nicer' project with no real 
user gains. I don't really see what it has to do with trying to get LyX 
running on Windows. In fact, producing something that works but is even 
clunkier than the existing code base (more #ifdefs) might provide even 
more motivation to refactor this piece of the code ;-)

Whereever this conversation goes, I don't think that any refactoring 
should be backported into the 1.3.x tree.

-- 
Angus

Re: The remainder of Ruurd's changes

[Jean-Marc Lasgouttes]

> "Lars" == Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:

| ?

Lars> No such thing.

What can unistd.h do that no  header does?

JMarc

Re: The remainder of Ruurd's changes

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| Jean-Marc Lasgouttes wrote:
>> A little digression: instead of our current os:: namespace, wouldn't
>> it be better to have a hierarchy of os_foo classes with static
>> methods, so that some environment could inherit others (like cygwin =
>> unix + some stuff). The some of the code from this init method could
>> be moved to os_foo.C files.
>
| You can't have static virtual member functions, but you could have either 
| a "class os" (à la 13x) or a "namespace os" (à la 14x) that hid the 
| implementation:
>
| class os {
| public:
| static void init(int argc, char * argv[]);
>
| static string const & binpath();
| static string const & binname();
| static string const & homepath();
| static string const & nulldev();
| private:
| struct impl {
| virtual string const & binpath() const = 0;
| virtual string const & binname() const = 0;
| virtual string const & homepath() const = 0;
| virtual string const & nulldev() const = 0;
| };
| static boost::scoped_ptr pimpl_;
| };

Will this work? Remember that you can call a static function without
the object.

(Ok I see what you want to do. (init should be private probably))

-- 
Lgb

The remainder of Ruurd's changes

[Angus Leeming]

As promised, I'm posting the remainder of the changes that are needed to
compile LyX with MinGW. This patch is much less intrusive than the
original because the offensive os_win32.h is how #included only by
those .C files that actually need it.

-- 
AngusIndex: src/ispell.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/ispell.C,v
retrieving revision 1.5.2.5
diff -u -p -r1.5.2.5 ispell.C
--- src/ispell.C	7 Dec 2004 10:48:23 -	1.5.2.5
+++ src/ispell.C	16 Dec 2004 01:04:41 -
@@ -21,12 +21,20 @@
 #include support/forkedcall.h
 #include support/lstrings.h
 
+#ifdef _WIN32
+# include support/os_win32.h
+#endif
+
 // HP-UX 11.x doesn't have this header
 #ifdef HAVE_SYS_SELECT_H
 #include sys/select.h
 #endif
 #include sys/time.h
 
+#ifdef HAVE_UNISTD_H
+# include unistd.h
+#endif
+
 #ifndef CXX_GLOBAL_CSTD
 using std::strcpy;
 using std::strlen;
@@ -309,11 +317,15 @@ bool ISpell::select(bool  err_read)
 	tv.tv_sec = 2;
 	tv.tv_usec = 0;
 
+#ifdef HAVE_SELECT
 	retval = ::select(SELECT_TYPE_ARG1 (max(pipeout[0], pipeerr[0]) + 1),
 			SELECT_TYPE_ARG234 (infds),
 			0,
 			0,
 			SELECT_TYPE_ARG5 (tv));
+#else
+	retval = -1;
+#endif
 
 	// error
 	if (retval = 0)
Index: src/lyx_cb.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyx_cb.C,v
retrieving revision 1.190
diff -u -p -r1.190 lyx_cb.C
--- src/lyx_cb.C	23 Jan 2003 16:23:37 -	1.190
+++ src/lyx_cb.C	16 Dec 2004 01:04:41 -
@@ -36,6 +36,10 @@
 #include support/systemcall.h
 #include support/lstrings.h
 
+#ifdef _WIN32
+# include support/os_win32.h
+#endif
+
 #include BoostFormat.h
 
 #include fstream
Index: src/lyx_main.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyx_main.C,v
retrieving revision 1.134.2.5
diff -u -p -r1.134.2.5 lyx_main.C
--- src/lyx_main.C	15 Dec 2004 19:35:10 -	1.134.2.5
+++ src/lyx_main.C	16 Dec 2004 01:04:42 -
@@ -38,6 +38,10 @@
 #include frontends/Alert.h
 #include frontends/lyx_gui.h
 
+#ifdef _WIN32
+# include support/os_win32.h
+#endif
+
 #include BoostFormat.h
 #include boost/function.hpp
 
@@ -289,7 +293,7 @@ void LyX::init(bool gui)
 	do {
 		// Path of binary/../share/name of binary/
 		searchpath += NormalizePath(AddPath(binpath, ../share/) +
-		  OnlyFilename(binname)) + ';';
+		  ChangeExtension(OnlyFilename(binname),)) + ';';
 
 		// Follow Symlinks
 		FileInfo file(fullbinpath, true);
Index: src/lyxserver.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyxserver.C,v
retrieving revision 1.48.2.1
diff -u -p -r1.48.2.1 lyxserver.C
--- src/lyxserver.C	7 Dec 2004 10:48:39 -	1.48.2.1
+++ src/lyxserver.C	16 Dec 2004 01:04:43 -
@@ -50,6 +50,10 @@
 #include support/lyxlib.h
 #include frontends/lyx_gui.h
 
+#ifdef _WIN32
+# include support/os_win32.h
+#endif
+
 #ifdef __EMX__
 #include cstdlib
 #include io.h
Index: src/frontends/qt2/qt_helpers.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/frontends/qt2/qt_helpers.C,v
retrieving revision 1.4.2.1
diff -u -p -r1.4.2.1 qt_helpers.C
--- src/frontends/qt2/qt_helpers.C	7 Dec 2004 10:49:15 -	1.4.2.1
+++ src/frontends/qt2/qt_helpers.C	16 Dec 2004 01:04:44 -
@@ -113,6 +113,7 @@ string const fromqstr(QString const  st
 {
 	QTextCodec * codec = QTextCodec::codecForLocale();
 	QCString tmpstr = codec-fromUnicode(str);
-	char const * tmpcstr = tmpstr;
+	char const * tmpcstr = \0;
+	if (!tmpstr.isEmpty()) tmpcstr = tmpstr;
 	return tmpcstr;
 }
Index: src/insets/insetgraphics.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetgraphics.C,v
retrieving revision 1.146.2.4
diff -u -p -r1.146.2.4 insetgraphics.C
--- src/insets/insetgraphics.C	7 Dec 2004 10:49:34 -	1.146.2.4
+++ src/insets/insetgraphics.C	16 Dec 2004 01:04:46 -
@@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
 		// without dots and again with ext
 		temp_file = ChangeExtension(
 			subst(temp_file, ., _), ext_tmp);
+		//Remove drive letter on Win32
+		if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);
 		// now we have any_dir_file.ext
 		temp_file = MakeAbsPath(temp_file, buf-tmppath);
 		lyxerr[Debug::GRAPHICS]
Index: src/support/FileInfo.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/FileInfo.C,v
retrieving revision 1.18.2.2
diff -u -p -r1.18.2.2 FileInfo.C
--- src/support/FileInfo.C	15 Dec 2004 21:40:03 -	1.18.2.2
+++ src/support/FileInfo.C	16 Dec 2004 01:04:46 -
@@ -10,8 +10,12 @@
 
 #include config.h
 
-//#include sys/types.h
-//#include sys/stat.h
+#include sys/types.h
+#include sys/stat.h
+
+#ifdef _WIN32
+# include os_win32.h
+#endif
 
 #include cerrno
 

The remainder of Ruurd's changes

[Angus Leeming]

As promised, I'm posting the remainder of the changes that are needed to
compile LyX with MinGW. This patch is much less intrusive than the
original because the offensive os_win32.h is how #included only by
those .C files that actually need it.

-- 
AngusIndex: src/ispell.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/ispell.C,v
retrieving revision 1.5.2.5
diff -u -p -r1.5.2.5 ispell.C
--- src/ispell.C	7 Dec 2004 10:48:23 -	1.5.2.5
+++ src/ispell.C	16 Dec 2004 01:04:41 -
@@ -21,12 +21,20 @@
 #include "support/forkedcall.h"
 #include "support/lstrings.h"
 
+#ifdef _WIN32
+# include "support/os_win32.h"
+#endif
+
 // HP-UX 11.x doesn't have this header
 #ifdef HAVE_SYS_SELECT_H
 #include 
 #endif
 #include 
 
+#ifdef HAVE_UNISTD_H
+# include 
+#endif
+
 #ifndef CXX_GLOBAL_CSTD
 using std::strcpy;
 using std::strlen;
@@ -309,11 +317,15 @@ bool ISpell::select(bool & err_read)
 	tv.tv_sec = 2;
 	tv.tv_usec = 0;
 
+#ifdef HAVE_SELECT
 	retval = ::select(SELECT_TYPE_ARG1 (max(pipeout[0], pipeerr[0]) + 1),
 			SELECT_TYPE_ARG234 (),
 			0,
 			0,
 			SELECT_TYPE_ARG5 ());
+#else
+	retval = -1;
+#endif
 
 	// error
 	if (retval <= 0)
Index: src/lyx_cb.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyx_cb.C,v
retrieving revision 1.190
diff -u -p -r1.190 lyx_cb.C
--- src/lyx_cb.C	23 Jan 2003 16:23:37 -	1.190
+++ src/lyx_cb.C	16 Dec 2004 01:04:41 -
@@ -36,6 +36,10 @@
 #include "support/systemcall.h"
 #include "support/lstrings.h"
 
+#ifdef _WIN32
+# include "support/os_win32.h"
+#endif
+
 #include "BoostFormat.h"
 
 #include 
Index: src/lyx_main.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyx_main.C,v
retrieving revision 1.134.2.5
diff -u -p -r1.134.2.5 lyx_main.C
--- src/lyx_main.C	15 Dec 2004 19:35:10 -	1.134.2.5
+++ src/lyx_main.C	16 Dec 2004 01:04:42 -
@@ -38,6 +38,10 @@
 #include "frontends/Alert.h"
 #include "frontends/lyx_gui.h"
 
+#ifdef _WIN32
+# include "support/os_win32.h"
+#endif
+
 #include "BoostFormat.h"
 #include 
 
@@ -289,7 +293,7 @@ void LyX::init(bool gui)
 	do {
 		// Path of binary/../share/name of binary/
 		searchpath += NormalizePath(AddPath(binpath, "../share/") +
-		  OnlyFilename(binname)) + ';';
+		  ChangeExtension(OnlyFilename(binname),"")) + ';';
 
 		// Follow Symlinks
 		FileInfo file(fullbinpath, true);
Index: src/lyxserver.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/lyxserver.C,v
retrieving revision 1.48.2.1
diff -u -p -r1.48.2.1 lyxserver.C
--- src/lyxserver.C	7 Dec 2004 10:48:39 -	1.48.2.1
+++ src/lyxserver.C	16 Dec 2004 01:04:43 -
@@ -50,6 +50,10 @@
 #include "support/lyxlib.h"
 #include "frontends/lyx_gui.h"
 
+#ifdef _WIN32
+# include "support/os_win32.h"
+#endif
+
 #ifdef __EMX__
 #include 
 #include 
Index: src/frontends/qt2/qt_helpers.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/frontends/qt2/qt_helpers.C,v
retrieving revision 1.4.2.1
diff -u -p -r1.4.2.1 qt_helpers.C
--- src/frontends/qt2/qt_helpers.C	7 Dec 2004 10:49:15 -	1.4.2.1
+++ src/frontends/qt2/qt_helpers.C	16 Dec 2004 01:04:44 -
@@ -113,6 +113,7 @@ string const fromqstr(QString const & st
 {
 	QTextCodec * codec = QTextCodec::codecForLocale();
 	QCString tmpstr = codec->fromUnicode(str);
-	char const * tmpcstr = tmpstr;
+	char const * tmpcstr = "\0";
+	if (!tmpstr.isEmpty()) tmpcstr = tmpstr;
 	return tmpcstr;
 }
Index: src/insets/insetgraphics.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetgraphics.C,v
retrieving revision 1.146.2.4
diff -u -p -r1.146.2.4 insetgraphics.C
--- src/insets/insetgraphics.C	7 Dec 2004 10:49:34 -	1.146.2.4
+++ src/insets/insetgraphics.C	16 Dec 2004 01:04:46 -
@@ -623,6 +623,8 @@ string const InsetGraphics::prepareFile(
 		// without dots and again with ext
 		temp_file = ChangeExtension(
 			subst(temp_file, ".", "_"), ext_tmp);
+		//Remove drive letter on Win32
+		if (temp_file[1] == ':') temp_file = temp_file.erase(0,2);
 		// now we have any_dir_file.ext
 		temp_file = MakeAbsPath(temp_file, buf->tmppath);
 		lyxerr[Debug::GRAPHICS]
Index: src/support/FileInfo.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/support/FileInfo.C,v
retrieving revision 1.18.2.2
diff -u -p -r1.18.2.2 FileInfo.C
--- src/support/FileInfo.C	15 Dec 2004 21:40:03 -	1.18.2.2
+++ src/support/FileInfo.C	16 Dec 2004 01:04:46 -
@@ -10,8 +10,12 @@
 
 #include 
 
-//#include 
-//#include 
+#include 
+#include 
+
+#ifdef _WIN32
+# include "os_win32.h"
+#endif
 
 #include 
 #include "FileInfo.h"
@@ -174,10 +178,20 @@ void FileInfo::init()
 
 void