Re: [PATCH] fix C4204 errors while building swig-py with Python 3.9 on Windows
On 16.11.2020 04:16, Jun Omae wrote:
Hi,
I tried to build swig-py with Python 3.9 on Windows, however get the
following errors:
[[[
C:\usr\apps\python39\include\cpython\abstract.h(205,1): error C4204:
nonstandard extension used: non-constant aggregate initializer
[C:\usr\src\subversion\trunk-py39\build\win32\vcnet-vcproj\libsvn_swig_py.vcxproj]
C:\usr\apps\python39\include\cpython\abstract.h(250,1): error C4204:
nonstandard extension used: non-constant aggregate initializer
[C:\usr\src\subversion\trunk-py39\build\win32\vcnet-vcproj\libsvn_swig_py.vcxproj]
]]]
The error was raised from the line 205 in cpython/abstract.h in Python
3.9:
[[[
202 static inline PyObject *
203 PyObject_CallMethodOneArg(PyObject *self, PyObject *name,
PyObject *arg)
204 {
205 PyObject *args[2] = {self, arg};
206
]]]
The attached patch adds #pragma warning to make C4204 a warning
instead of an
error only on building swig-py with Visual Studio.
+1
Re: svn commit: r1881534 - /subversion/trunk/build/ac-macros/macosx.m4
On 15.11.2020 19:16, Nathan Hartman wrote: On Sat, Nov 14, 2020 at 4:51 AM Branko Čibej wrote: On 13.11.2020 17:37, Nathan Hartman wrote: Done in r1883388. Added to the backport proposal but I'll let you un-veto it :-) This fixes the issue on trunk, but when I merge the two revisions to 1.14.x ... $ svn diff svn: E135000: File '.../build/ac-macros/macosx.m4' has inconsistent newlines svn: E135000: Inconsistent line ending style $ svn ps svn:eol-style native build/ac-macros/macosx.m4 svn: E29: File '.../build/ac-macros/macosx.m4' has inconsistent newlines svn: E135000: Inconsistent line ending style Did we really mean this? that 'svn diff' errors out on inconsistent newlines? Really really? And that you can't make them consistent by setting the svn:eol-style property (which 'svn merge' already did')? Looks like a bug to me. I'll try to track it down... It has something to do with eol-style processing. I also get "E200042: Additional errors:" but there are no additional errors listed. Same result happens regardless of the order of merging the two revisions, but removing the svn-eol-style property makes 'svn diff' work normally. Looks like this behaviour has been in the code for a long time. It's correct that 'svn diff' doesn't try to "repair" the line endings, those must show up in the diff, but failing is kind of wrong. It's correct for 'svn commit' iff we have svn:eol-style set, but IMO wrong for 'svn diff'. Also I *do not* understand how that could even be an issue in this case, since the line endings were fixed in the repository. Does 'svn merge' do something wrong? -- Brane
Re: [Patch] Update the INSTALL file for SWIG bindings (Re: svn commit: r1883333 - /subversion/site/staging/docs/release-notes/1.14.html)
Yasuhito FUTATSUKI wrote on Sat, 14 Nov 2020 14:52 +0900: > +++ subversion/bindings/swig/INSTALL (working copy) > @@ -141,7 +141,15 @@ > - Make sure that Subversion's ./configure script sees your installed SWIG! > + If you are using the distribution tarball and you want to use the language > + bindings C source files shipped with it, you might need to pass the > + --without-swig option to configure script to avoid detecting and checking > + SWIG on your system. A Makefile generated by configure will prevent > + building the language bindings if the configure script detect unsuitable > + version of SWIG. I don't dispute the accuracy of this paragraph, but I think this API isn't autotools-idiomatic. Generally, I'd expect --without-foo to short-circuit the probe for foo and assume foo isn't found; i.e., if my system has an unsuitable version of foo, I'd the default behaviour (given neither --with-foo nor --without-foo) and the behaviour given --without-foo to be identical: namely, behave as though foo isn't available (even if /usr/bin/foo exists and is perfectly suitable). In particular, if my system has an unsuitable version of swig, I wouldn't expect passing --without-swig to change configure's behaviour. Cheers, Daniel
Re: svn commit: r1881534 - /subversion/trunk/build/ac-macros/macosx.m4
Branko Čibej wrote on Mon, 16 Nov 2020 16:15 +0100: > On 15.11.2020 19:16, Nathan Hartman wrote: > > On Sat, Nov 14, 2020 at 4:51 AM Branko Čibej wrote: > >> On 13.11.2020 17:37, Nathan Hartman wrote: > >>> Done in r1883388. Added to the backport proposal but I'll let you un-veto > >>> it :-) > >> > >> This fixes the issue on trunk, but when I merge the two revisions to > >> 1.14.x ... > >> > >> $ svn diff > >> svn: E135000: File '.../build/ac-macros/macosx.m4' has inconsistent > >> newlines > >> svn: E135000: Inconsistent line ending style > >> > >> $ svn ps svn:eol-style native build/ac-macros/macosx.m4 > >> svn: E29: File '.../build/ac-macros/macosx.m4' has inconsistent > >> newlines > >> svn: E135000: Inconsistent line ending style > >> > >> > >> Did we really mean this? that 'svn diff' errors out on inconsistent > >> newlines? Really really? And that you can't make them consistent by > >> setting the svn:eol-style property (which 'svn merge' already did')? > > Looks like a bug to me. I'll try to track it down... It has something > > to do with eol-style processing. I also get "E200042: Additional > > errors:" but there are no additional errors listed. Same result > > happens regardless of the order of merging the two revisions, but > > removing the svn-eol-style property makes 'svn diff' work normally. > > > Looks like this behaviour has been in the code for a long time. It's > correct that 'svn diff' doesn't try to "repair" the line endings, those > must show up in the diff, but failing is kind of wrong. It's correct for > 'svn commit' iff we have svn:eol-style set, but IMO wrong for 'svn diff'. > > Also I *do not* understand how that could even be an issue in this case, > since the line endings were fixed in the repository. Does 'svn merge' do > something wrong? The file ends up with svn:eol-style=native set (even before the propset) but a mix of LF and CRLF on disk. Passing it through dos2unix(1) makes the error go away. Seems like several different bugs: - Merging the propset left the file with inconsistent newlines. - «svn diff» treated the inconsistency as a fatal error, even under -x--ignore-eol-style. It could have printed a diff (plus or minus a warning, when -x--ignore-eol-style isn't in effect). - brane's idempotent «propset» failed. Cheers, Daniel
Re: [PATCH] fix C4204 errors while building swig-py with Python 3.9 on Windows
Branko Čibej wrote on Mon, 16 Nov 2020 09:19 +0100: > On 16.11.2020 04:16, Jun Omae wrote: > > The attached patch adds #pragma warning to make C4204 a warning > > instead of an > > error only on building swig-py with Visual Studio. > > +1 Should that be added to build/generator/templates/ instead? Note that 4204 is specifically there in two places already.
Re: [Patch] Update the INSTALL file for SWIG bindings (Re: svn commit: r1883333 - /subversion/site/staging/docs/release-notes/1.14.html)
On 2020/11/17 2:16, Daniel Shahaf wrote: > Yasuhito FUTATSUKI wrote on Sat, 14 Nov 2020 14:52 +0900: >> +++ subversion/bindings/swig/INSTALL (working copy) >> @@ -141,7 +141,15 @@ >> - Make sure that Subversion's ./configure script sees your installed SWIG! >> + If you are using the distribution tarball and you want to use the language >> + bindings C source files shipped with it, you might need to pass the >> + --without-swig option to configure script to avoid detecting and checking >> + SWIG on your system. A Makefile generated by configure will prevent >> + building the language bindings if the configure script detect unsuitable >> + version of SWIG. > > I don't dispute the accuracy of this paragraph, but I think this API > isn't autotools-idiomatic. > > Generally, I'd expect --without-foo to short-circuit the probe for foo > and assume foo isn't found; i.e., if my system has an unsuitable > version of foo, I'd the default behaviour (given neither --with-foo nor > --without-foo) and the behaviour given --without-foo to be identical: > namely, behave as though foo isn't available (even if /usr/bin/foo > exists and is perfectly suitable). > > In particular, if my system has an unsuitable version of swig, > I wouldn't expect passing --without-swig to change configure's behaviour. Probably what is wrong here is that the configure script accepts --with-swig | --without-swig options and checks it in release mode. We never clean SWIG generated language bindings C source files on clean-foo targets in release mode Makefile. extraclean-foo targets do it, but they are only parts of the extraclean target which also removes all release mode stuff. So users never use SWIG in release mode actually. That is, r1876662 is not correct. Cheers, -- Yasuhito FUTATSUKI
Re: [PATCH] fix C4204 errors while building swig-py with Python 3.9 on Windows
On 2020/11/17 2:33, Daniel Shahaf wrote: > Branko Čibej wrote on Mon, 16 Nov 2020 09:19 +0100: >> On 16.11.2020 04:16, Jun Omae wrote: >>> The attached patch adds #pragma warning to make C4204 a warning >>> instead of an >>> error only on building swig-py with Visual Studio. >> >> +1 +1 > Should that be added to build/generator/templates/ instead? Note that > 4204 is specifically there in two places already. I think this fix is better than the fix by removing 4204 from the list of warnings that are treated as error in build/generator/templates/. This is like a situation with the C4115 case last year [1]. As the problem is not in our source code, we wanted to relax the compile option only in SWIG Python bindings, but we couldn't immediately. So we removed C4115 from the list as a compromise. [1] https://lists.apache.org/thread.html/f1f53e2a8710b6478ca8fd0efa68907e8641ba77afdf64d9f208d85f%40%3Cdev.subversion.apache.org%3E Cheers, -- Yasuhito FUTATSUKI
Re: [PATCH] fix C4204 errors while building swig-py with Python 3.9 on Windows
On Tue, Nov 17, 2020 at 2:33 AM Daniel Shahaf wrote: > > Branko Čibej wrote on Mon, 16 Nov 2020 09:19 +0100: > > On 16.11.2020 04:16, Jun Omae wrote: > > > The attached patch adds #pragma warning to make C4204 a warning > > > instead of an > > > error only on building swig-py with Visual Studio. > > > > +1 > > Should that be added to build/generator/templates/ instead? Note that > 4204 is specifically there in two places already. The C4204 has turned into an error in r1442063. I've searched reasons of r1442063 in the archive of dev@ mailing list before posting the patch, however it was unable to find. I consider that r1442063 is to detect breakage of C89/C90 compat, the proposed patch has no changes of build/generator/templates/*.ezt files. -- Jun Omae (大前 潤)
