Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory

[Jon Turney <jon.turney@dronecode.org.uk>]

On 06/05/2021 09:43, Corinna Vinschen wrote:
> On May  4 19:34, Jon Turney wrote:
>> On 03/05/2021 11:48, Corinna Vinschen wrote:
>>> On May  2 16:25, Jon Turney wrote:
>>>> Move all the source files used in utils/mingw/ into that subdirectory,
>>>> so the built objects are in the expected place.
>>>>
>>>> (path.cc requires some more unpicking, and even then there is genuinely
>>>> some shared code, so use a trivial file which includes the real path.cc
>>>> so the object file is generated where expected)
>>>
>>> This patchset LGTM, except one thing which isn't your fault:
>>>
>>>> index b96ad40c1..a7797600c 100644
>>>> --- a/winsup/utils/strace.cc
>>>> +++ b/winsup/utils/mingw/strace.cc
>>>> @@ -21,11 +21,11 @@ details. */
>>>>    #include <time.h>
>>>>    #include <signal.h>
>>>>    #include <errno.h>
>>>> -#include "../cygwin/include/sys/strace.h"
>>>> -#include "../cygwin/include/sys/cygwin.h"
>>>> -#include "../cygwin/include/cygwin/version.h"
>>>> -#include "../cygwin/cygtls_padsize.h"
>>>> -#include "../cygwin/gcc_seh.h"
>>>> +#include "../../cygwin/include/sys/strace.h"
>>>> +#include "../../cygwin/include/sys/cygwin.h"
>>>> +#include "../../cygwin/include/cygwin/version.h"
>>>> +#include "../../cygwin/cygtls_padsize.h"
>>>> +#include "../../cygwin/gcc_seh.h"
>>>
>>> What about adding -I../../cygwin -I../../cygwin/include to the build
>>> rules and get rid of the relative paths inside the sources?
>>
>> That seems fraught as it allows cygwin system headers to be picked up in
>> preference to mingw ones?
>>
>> Using '-idirafter' gets you a build, but it would be much more work to check
>> that you've actually built what you wanted to...
> 
> Well, ok.  It just looks *so* ugly...  What about at least
> 
>    --idirafter ../../cygwin
> 
> and then
> 
>        #include "include/sys/strace.h"
>        #include "include/sys/cygwin.h"
>        #include "include/cygwin/version.h"
>        #include "cygtls_padsize.h"
>        #include "gcc_seh.h"
>    
> That would disallow picking up system headers and still be a bit
> cleaner, no?

After thinking about this a bit more, I'm fairly certain that using 
-idirafter with both paths gets us the same build as before, so I've 
posted a patch with that change.

However, as written it's still a bit dangerous: any includes of system 
headers by those files included from winsup/cygwin will be getting MinGW 
system headers. I don't think that e.g. the value of ULONG_MAX is going 
be used by any of those, but there is a theoretical risk of them not 
getting what is expected...

Perhaps the only safe way to write this is to put the numeric constants 
which strace uses into a separate header.