* [PATCH 0/2] Improve automake object file location for utils/mingw/
@ 2021-05-02 15:25 Jon Turney
2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jon Turney @ 2021-05-02 15:25 UTC (permalink / raw)
To: cygwin-patches; +Cc: Jon Turney
This is the simple part, leaving potentially pulling apart path.cc for later...
Jon Turney (2):
Unpick cygpath TESTSUITE
Move source files used in utils/mingw/ into that subdirectory
winsup/utils/mingw/Makefile.am | 23 +++++++------
winsup/utils/{ => mingw}/bloda.cc | 0
winsup/utils/{ => mingw}/cygcheck.cc | 0
.../{ => mingw}/cygwin-console-helper.cc | 0
winsup/utils/{ => mingw}/dump_setup.cc | 0
winsup/utils/{ => mingw}/ldh.cc | 0
winsup/utils/mingw/path.cc | 1 +
winsup/utils/{ => mingw}/strace.cc | 10 +++---
winsup/utils/{ => mingw}/testsuite.cc | 31 +++++++++--------
winsup/utils/{ => mingw}/testsuite.h | 34 ++++++-------------
winsup/utils/path.cc | 31 +++++++----------
winsup/utils/path.h | 10 ++++--
12 files changed, 67 insertions(+), 73 deletions(-)
rename winsup/utils/{ => mingw}/bloda.cc (100%)
rename winsup/utils/{ => mingw}/cygcheck.cc (100%)
rename winsup/utils/{ => mingw}/cygwin-console-helper.cc (100%)
rename winsup/utils/{ => mingw}/dump_setup.cc (100%)
rename winsup/utils/{ => mingw}/ldh.cc (100%)
create mode 100644 winsup/utils/mingw/path.cc
rename winsup/utils/{ => mingw}/strace.cc (99%)
rename winsup/utils/{ => mingw}/testsuite.cc (85%)
rename winsup/utils/{ => mingw}/testsuite.h (94%)
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] Unpick cygpath TESTSUITE
2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney
@ 2021-05-02 15:25 ` Jon Turney
2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney
2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney
2 siblings, 0 replies; 10+ messages in thread
From: Jon Turney @ 2021-05-02 15:25 UTC (permalink / raw)
To: cygwin-patches; +Cc: Jon Turney
Rather than having testsuite.h do various things, depending on defines,
just have it do one thing, and then explicitly redirect to test stubs in
path.cc when building test.
---
winsup/utils/path.cc | 31 +++++++++++++------------------
winsup/utils/path.h | 10 ++++++++--
winsup/utils/testsuite.cc | 31 ++++++++++++++++---------------
winsup/utils/testsuite.h | 34 +++++++++++-----------------------
4 files changed, 48 insertions(+), 58 deletions(-)
diff --git a/winsup/utils/path.cc b/winsup/utils/path.cc
index 29344be02..0d41a45d8 100644
--- a/winsup/utils/path.cc
+++ b/winsup/utils/path.cc
@@ -25,7 +25,6 @@ details. */
#include "../cygwin/include/sys/mount.h"
#define _NOMNTENT_MACROS
#include "../cygwin/include/mntent.h"
-#include "testsuite.h"
#ifdef FSTAB_ONLY
#include <sys/cygwin.h>
#endif
@@ -255,14 +254,8 @@ readlink (HANDLE fh, char *path, size_t maxlen)
}
#endif /* !FSTAB_ONLY */
-#ifndef TESTSUITE
mnt_t mount_table[255];
int max_mount_entry;
-#else
-# define TESTSUITE_MOUNT_TABLE
-# include "testsuite.h"
-# undef TESTSUITE_MOUNT_TABLE
-#endif
inline void
unconvert_slashes (char* name)
@@ -271,9 +264,6 @@ unconvert_slashes (char* name)
*name++ = '\\';
}
-/* These functions aren't called when defined(TESTSUITE) which results
- in a compiler warning. */
-#ifndef TESTSUITE
inline char *
skip_ws (char *in)
{
@@ -555,11 +545,8 @@ from_fstab (bool user, PWCHAR path, PWCHAR path_end)
CloseHandle (h);
}
#endif /* !FSTAB_ONLY */
-#endif /* !TESTSUITE */
#ifndef FSTAB_ONLY
-
-#ifndef TESTSUITE
static int
mnt_sort (const void *a, const void *b)
{
@@ -653,7 +640,11 @@ read_mounts ()
from_fstab (true, path, path_end);
qsort (mount_table, max_mount_entry, sizeof (mnt_t), mnt_sort);
}
-#endif /* !defined(TESTSUITE) */
+
+#ifdef TESTSUITE
+#define read_mounts testsuite_read_mounts
+#endif
+
/* Return non-zero if PATH1 is a prefix of PATH2.
Both are assumed to be of the same path style and / vs \ usage.
@@ -757,6 +748,11 @@ concat (const char *s, ...)
return vconcat (s, v);
}
+#ifdef TESTSUITE
+#undef GetCurrentDirectory
+#define GetCurrentDirectory testsuite_getcwd
+#endif
+
/* This is a helper function for when vcygpath is passed what appears
to be a relative POSIX path. We take a Win32 CWD (either as specified
in 'cwd' or as retrieved with GetCurrentDirectory() if 'cwd' is NULL)
@@ -822,10 +818,9 @@ vcygpath (const char *cwd, const char *s, va_list v)
size_t max_len = 0;
mnt_t *m, *match = NULL;
-#ifndef TESTSUITE
if (!max_mount_entry)
read_mounts ();
-#endif
+
char *path;
if (s[0] == '.' && isslash (s[1]))
s += 2;
@@ -912,10 +907,10 @@ extern "C" FILE *
setmntent (const char *, const char *)
{
m = mount_table;
-#ifndef TESTSUITE
+
if (!max_mount_entry)
read_mounts ();
-#endif
+
return NULL;
}
diff --git a/winsup/utils/path.h b/winsup/utils/path.h
index a1840a003..c64f6ebfb 100644
--- a/winsup/utils/path.h
+++ b/winsup/utils/path.h
@@ -6,6 +6,9 @@ This software is a copyrighted work licensed under the terms of the
Cygwin license. Please consult the file "CYGWIN_LICENSE" for
details. */
+#ifndef _PATH_H_
+#define _PATH_H_
+
struct mnt_t
{
char *native;
@@ -22,11 +25,14 @@ int get_word (HANDLE, int);
int get_dword (HANDLE, int);
bool from_fstab_line (mnt_t *m, char *line, bool user);
-#ifndef TESTSUITE
extern mnt_t mount_table[255];
extern int max_mount_entry;
-#endif
#ifndef SYMLINK_MAX
#define SYMLINK_MAX 4095 /* PATH_MAX - 1 */
#endif
+
+DWORD testsuite_getcwd (DWORD nBufferLength, LPSTR lpBuffer);
+void testsuite_read_mounts (void);
+
+#endif
diff --git a/winsup/utils/testsuite.cc b/winsup/utils/testsuite.cc
index 23ed8e0d8..ef9f14fa7 100644
--- a/winsup/utils/testsuite.cc
+++ b/winsup/utils/testsuite.cc
@@ -15,22 +15,9 @@ details. */
#include <unistd.h>
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
-#ifndef TESTSUITE
-#define TESTSUITE
-#endif
+#include "path.h"
#include "testsuite.h"
-typedef struct
- {
- const char *cwd; /* in win32 form, as if by GetCurrentDirectory */
- const char *posix; /* input */
- const char *win32; /* expected output */
- } test_t;
-
-#define TESTSUITE_TESTS
-#include "testsuite.h"
-#undef TESTSUITE_TESTS
-
static int curtest;
/* A replacement for the w32api GetCurrentDirectory() that returns
@@ -55,7 +42,21 @@ testsuite_getcwd (DWORD nBufferLength, LPSTR lpBuffer)
return len;
}
-extern char *cygpath (const char *s, ...);
+/*
+ A replacement for read_mounts that installs the test mount table
+ */
+void
+testsuite_read_mounts (void)
+{
+ int i;
+ for (i = 0; test_mount_table[i].posix; i++)
+ {
+ mount_table[i] = test_mount_table[i];
+ }
+ max_mount_entry = i;
+}
+
+WCHAR cygwin_dll_path[32768];
int
main (int argc, char **argv)
diff --git a/winsup/utils/testsuite.h b/winsup/utils/testsuite.h
index 0dd631539..c3d9ad60d 100644
--- a/winsup/utils/testsuite.h
+++ b/winsup/utils/testsuite.h
@@ -6,6 +6,10 @@ This software is a copyrighted work licensed under the terms of the
Cygwin license. Please consult the file "CYGWIN_LICENSE" for
details. */
+#include "path.h"
+#include "../cygwin/include/cygwin/bits.h"
+#include "../cygwin/include/sys/mount.h"
+
/* This file implements a test harness for the MinGW implementation of
POSIX path translation in utils/path.cc. This code is used by strace
and cygcheck which cannot depend on the Cygwin DLL. The tests below
@@ -13,9 +17,6 @@ details. */
absolute paths from POSIX form to Win32 form based on the contents of
a mount table. */
-/* Including this file should be a no-op if TESTSUITE is not defined. */
-#ifdef TESTSUITE
-
/* These definitions are common to both the testsuite mount table
as well as the testsuite definitions themselves, so define them
here so that they are only defined in one location. */
@@ -26,9 +27,7 @@ details. */
This is used in place of actually reading the host mount
table from the registry for the duration of the testsuite. This
table should match the battery of tests below. */
-
-#if defined(TESTSUITE_MOUNT_TABLE)
-static mnt_t mount_table[] = {
+static mnt_t test_mount_table[] = {
/* native posix flags */
{ (char*)TESTSUITE_ROOT, (char*)"/", MOUNT_SYSTEM},
{ (char*)"O:\\other", (char*)"/otherdir", MOUNT_SYSTEM},
@@ -39,12 +38,16 @@ static mnt_t mount_table[] = {
{ NULL, (char*)NULL, 0}
};
+typedef struct
+ {
+ const char *cwd; /* in win32 form, as if by GetCurrentDirectory */
+ const char *posix; /* input */
+ const char *win32; /* expected output */
+ } test_t;
/* Define the main set of tests. This is defined here instead of in
testsuite.cc so that all test harness data is in one place and not
spread over several files. */
-
-#elif defined(TESTSUITE_TESTS)
#define NO_CWD "N/A"
static test_t testsuite_tests[] = {
{ NO_CWD, "/file.ext", TESTSUITE_ROOT"\\file.ext" },
@@ -112,18 +115,3 @@ static test_t testsuite_tests[] = {
{ NO_CWD, "//server/share/foo/bar", "\\\\server\\share\\foo\\bar" },
{ NO_CWD, NULL, NULL }
};
-
-#else
-
-/* Redirect calls to GetCurrentDirectory() to the testsuite instead. */
-#ifdef GetCurrentDirectory
-#undef GetCurrentDirectory
-#endif
-#define GetCurrentDirectory testsuite_getcwd
-
-DWORD testsuite_getcwd (DWORD, LPSTR);
-
-#endif
-
-#endif /* TESTSUITE */
-
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory
2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney
2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney
@ 2021-05-02 15:25 ` Jon Turney
2021-05-03 10:48 ` Corinna Vinschen
2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney
2 siblings, 1 reply; 10+ messages in thread
From: Jon Turney @ 2021-05-02 15:25 UTC (permalink / raw)
To: cygwin-patches; +Cc: Jon Turney
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)
---
winsup/utils/mingw/Makefile.am | 23 +++++++++++--------
winsup/utils/{ => mingw}/bloda.cc | 0
winsup/utils/{ => mingw}/cygcheck.cc | 0
.../{ => mingw}/cygwin-console-helper.cc | 0
winsup/utils/{ => mingw}/dump_setup.cc | 0
winsup/utils/{ => mingw}/ldh.cc | 0
winsup/utils/mingw/path.cc | 1 +
winsup/utils/{ => mingw}/strace.cc | 10 ++++----
winsup/utils/{ => mingw}/testsuite.cc | 0
winsup/utils/{ => mingw}/testsuite.h | 0
10 files changed, 19 insertions(+), 15 deletions(-)
rename winsup/utils/{ => mingw}/bloda.cc (100%)
rename winsup/utils/{ => mingw}/cygcheck.cc (100%)
rename winsup/utils/{ => mingw}/cygwin-console-helper.cc (100%)
rename winsup/utils/{ => mingw}/dump_setup.cc (100%)
rename winsup/utils/{ => mingw}/ldh.cc (100%)
create mode 100644 winsup/utils/mingw/path.cc
rename winsup/utils/{ => mingw}/strace.cc (99%)
rename winsup/utils/{ => mingw}/testsuite.cc (100%)
rename winsup/utils/{ => mingw}/testsuite.h (100%)
diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am
index a14725902..73abc4264 100644
--- a/winsup/utils/mingw/Makefile.am
+++ b/winsup/utils/mingw/Makefile.am
@@ -25,26 +25,29 @@ bin_PROGRAMS = \
strace
cygcheck_SOURCES = \
- ../bloda.cc \
- ../cygcheck.cc \
- ../dump_setup.cc \
- ../path.cc
+ bloda.cc \
+ cygcheck.cc \
+ dump_setup.cc \
+ path.cc
+cygcheck_CPPFLAGS=-I$(srcdir)/..
cygcheck_LDADD = -lz -lwininet -lpsapi -lntdll
-cygwin_console_helper_SOURCES = ../cygwin-console-helper.cc
+cygwin_console_helper_SOURCES = cygwin-console-helper.cc
-ldh_SOURCES = ../ldh.cc
+ldh_SOURCES = ldh.cc
strace_SOURCES = \
- ../path.cc \
- ../strace.cc
+ path.cc \
+ strace.cc
+strace_CPPFLAGS=-I$(srcdir)/..
strace_LDADD = -lntdll
noinst_PROGRAMS = path-testsuite
path_testsuite_SOURCES = \
- ../path.cc \
- ../testsuite.cc
+ path.cc \
+ testsuite.cc
+path_testsuite_CPPFLAGS=-I$(srcdir)/..
path_testsuite_CXXFLAGS = -DTESTSUITE
TESTS = path-testsuite
diff --git a/winsup/utils/bloda.cc b/winsup/utils/mingw/bloda.cc
similarity index 100%
rename from winsup/utils/bloda.cc
rename to winsup/utils/mingw/bloda.cc
diff --git a/winsup/utils/cygcheck.cc b/winsup/utils/mingw/cygcheck.cc
similarity index 100%
rename from winsup/utils/cygcheck.cc
rename to winsup/utils/mingw/cygcheck.cc
diff --git a/winsup/utils/cygwin-console-helper.cc b/winsup/utils/mingw/cygwin-console-helper.cc
similarity index 100%
rename from winsup/utils/cygwin-console-helper.cc
rename to winsup/utils/mingw/cygwin-console-helper.cc
diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/mingw/dump_setup.cc
similarity index 100%
rename from winsup/utils/dump_setup.cc
rename to winsup/utils/mingw/dump_setup.cc
diff --git a/winsup/utils/ldh.cc b/winsup/utils/mingw/ldh.cc
similarity index 100%
rename from winsup/utils/ldh.cc
rename to winsup/utils/mingw/ldh.cc
diff --git a/winsup/utils/mingw/path.cc b/winsup/utils/mingw/path.cc
new file mode 100644
index 000000000..dd275b61d
--- /dev/null
+++ b/winsup/utils/mingw/path.cc
@@ -0,0 +1 @@
+#include "../path.cc"
diff --git a/winsup/utils/strace.cc b/winsup/utils/mingw/strace.cc
similarity index 99%
rename from winsup/utils/strace.cc
rename to winsup/utils/mingw/strace.cc
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"
#include "path.h"
#undef cygwin_internal
#include "loadlib.h"
diff --git a/winsup/utils/testsuite.cc b/winsup/utils/mingw/testsuite.cc
similarity index 100%
rename from winsup/utils/testsuite.cc
rename to winsup/utils/mingw/testsuite.cc
diff --git a/winsup/utils/testsuite.h b/winsup/utils/mingw/testsuite.h
similarity index 100%
rename from winsup/utils/testsuite.h
rename to winsup/utils/mingw/testsuite.h
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory
2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney
@ 2021-05-03 10:48 ` Corinna Vinschen
2021-05-04 18:34 ` Jon Turney
0 siblings, 1 reply; 10+ messages in thread
From: Corinna Vinschen @ 2021-05-03 10:48 UTC (permalink / raw)
To: cygwin-patches
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?
Thanks,
Corinna
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory
2021-05-03 10:48 ` Corinna Vinschen
@ 2021-05-04 18:34 ` Jon Turney
2021-05-06 8:43 ` Corinna Vinschen
0 siblings, 1 reply; 10+ messages in thread
From: Jon Turney @ 2021-05-04 18:34 UTC (permalink / raw)
To: Cygwin Patches
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...
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory
2021-05-04 18:34 ` Jon Turney
@ 2021-05-06 8:43 ` Corinna Vinschen
2021-05-09 15:16 ` Jon Turney
0 siblings, 1 reply; 10+ messages in thread
From: Corinna Vinschen @ 2021-05-06 8:43 UTC (permalink / raw)
To: cygwin-patches
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?
Corinna
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/2] Get rid of relative include paths in strace.cc
2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney
2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney
2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney
@ 2021-05-09 15:09 ` Jon Turney
2021-05-10 8:09 ` Corinna Vinschen
2 siblings, 1 reply; 10+ messages in thread
From: Jon Turney @ 2021-05-09 15:09 UTC (permalink / raw)
To: cygwin-patches; +Cc: Jon Turney
---
winsup/utils/mingw/Makefile.am | 2 +-
winsup/utils/mingw/strace.cc | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am
index 73abc4264..874dce512 100644
--- a/winsup/utils/mingw/Makefile.am
+++ b/winsup/utils/mingw/Makefile.am
@@ -39,7 +39,7 @@ ldh_SOURCES = ldh.cc
strace_SOURCES = \
path.cc \
strace.cc
-strace_CPPFLAGS=-I$(srcdir)/..
+strace_CPPFLAGS=-I$(srcdir)/.. -idirafter ${top_srcdir}/cygwin -idirafter ${top_srcdir}/cygwin/include
strace_LDADD = -lntdll
noinst_PROGRAMS = path-testsuite
diff --git a/winsup/utils/mingw/strace.cc b/winsup/utils/mingw/strace.cc
index a7797600c..d8a095fb6 100644
--- a/winsup/utils/mingw/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 "sys/strace.h"
+#include "sys/cygwin.h"
+#include "cygwin/version.h"
+#include "cygtls_padsize.h"
+#include "gcc_seh.h"
#include "path.h"
#undef cygwin_internal
#include "loadlib.h"
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory
2021-05-06 8:43 ` Corinna Vinschen
@ 2021-05-09 15:16 ` Jon Turney
2021-05-10 7:51 ` Corinna Vinschen
0 siblings, 1 reply; 10+ messages in thread
From: Jon Turney @ 2021-05-09 15:16 UTC (permalink / raw)
To: Cygwin Patches
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory
2021-05-09 15:16 ` Jon Turney
@ 2021-05-10 7:51 ` Corinna Vinschen
0 siblings, 0 replies; 10+ messages in thread
From: Corinna Vinschen @ 2021-05-10 7:51 UTC (permalink / raw)
To: cygwin-patches
On May 9 16:16, Jon Turney wrote:
> 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:
> > > > 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.
That sounds like a really good idea.
Corinna
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/2] Get rid of relative include paths in strace.cc
2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney
@ 2021-05-10 8:09 ` Corinna Vinschen
0 siblings, 0 replies; 10+ messages in thread
From: Corinna Vinschen @ 2021-05-10 8:09 UTC (permalink / raw)
To: cygwin-patches
On May 9 16:09, Jon Turney wrote:
> ---
> winsup/utils/mingw/Makefile.am | 2 +-
> winsup/utils/mingw/strace.cc | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am
> index 73abc4264..874dce512 100644
> --- a/winsup/utils/mingw/Makefile.am
> +++ b/winsup/utils/mingw/Makefile.am
> @@ -39,7 +39,7 @@ ldh_SOURCES = ldh.cc
> strace_SOURCES = \
> path.cc \
> strace.cc
> -strace_CPPFLAGS=-I$(srcdir)/..
> +strace_CPPFLAGS=-I$(srcdir)/.. -idirafter ${top_srcdir}/cygwin -idirafter ${top_srcdir}/cygwin/include
> strace_LDADD = -lntdll
>
> noinst_PROGRAMS = path-testsuite
> diff --git a/winsup/utils/mingw/strace.cc b/winsup/utils/mingw/strace.cc
> index a7797600c..d8a095fb6 100644
> --- a/winsup/utils/mingw/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 "sys/strace.h"
> +#include "sys/cygwin.h"
> +#include "cygwin/version.h"
> +#include "cygtls_padsize.h"
> +#include "gcc_seh.h"
> #include "path.h"
> #undef cygwin_internal
> #include "loadlib.h"
> --
> 2.31.1
Great, please push.
Thanks,
Corinna
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-05-10 8:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney
2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney
2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney
2021-05-03 10:48 ` Corinna Vinschen
2021-05-04 18:34 ` Jon Turney
2021-05-06 8:43 ` Corinna Vinschen
2021-05-09 15:16 ` Jon Turney
2021-05-10 7:51 ` Corinna Vinschen
2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney
2021-05-10 8:09 ` Corinna Vinschen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).