GNOME Bugzilla – Bug 620566
[build] Win32 support
Last modified: 2018-02-08 11:55:19 UTC
Created attachment 162747 [details] [review] outdated patch from Tor The win32 build of gobject-introspection is broken
Created attachment 195954 [details] [review] fix _resolve_non_libtool
Created attachment 195955 [details] [review] replace a stray backslash with a slash. On Monday, August 29 2011, jdahlin wrote: > use os.path.join or os.sep Out of experience I know this is needed as nothing works without the .replace(), but I don't immediately see how this can be done with os.path.join?
Created attachment 195956 [details] [review] Ensure ld can locate the libraries the temporary binary links On Monday, August 29 2011, jdahlin wrote: > > Why ../bin? Because that's where they (.dll's) live on windows.
Created attachment 195957 [details] [review] fix Python shebang lines This one is a bit controversial I guess... On Monday, August 29 2011, jdahlin wrote: > Why can't you use the templates on windows? This looks wrong. Because whatever the template resolves to is only valid on the build system (or the temporary prefixes it uses to construct such a build system). For example (in MSYS speak): /d/bin/Python27/python might be valid on my machine but will most likely not exist on yours where it could be /c/Python27/python or something else entirely. Had the same discussion with Colin Walters in bug #650763 (caution, it's a huge thread). And having it again in bug #658237. I won't be terribly disappointed if this is rejected, I could just as well adjust my build system to sed @PYTHON@ before make. But having a proper fix upstream would be better of course...
Created attachment 195958 [details] [review] Make g-ir-annotiotion-tool, g-ir-doc-tool and g-ir-scanner 'relocatable' at runtime.
Created attachment 195959 [details] [review] Monkeypatch subprocess.Popen
Created attachment 195960 [details] [review] Monkeypatch os.path.join
Created attachment 195961 [details] [review] Windows port: pass arguments through a file On Monday, August 29 2011, jdahlin wrote: > If you're adding the file name path you need to make this work on non-win32 as well Hmm... Currently the args_file option is only available when the 'MSYSTEM' environment variable == 'MINGW32' [1]. Do you really want this to become a generic option available on other platforms? [1] Set when running an MSYS bash session in 'MinGW' mode, so when we want to build native Windows binaries. The MSYSTEM environment variable can also be set to 'MSYS' when running in 'MSYS System Builder' mode. That's when you want to build windows binaries linked against msys-1.0.dll (things that need to be aware of the MSYS Posix environment like bash itself for example), which is clearly not the case here...
Created attachment 195962 [details] [review] [WIP] Windows port: pass arguments through a file This needs to be done on MinGW/MSYS only, but how? On Monday, August 29 2011, jdahlin wrote: > This is unfortunate, I saw the reasoning a bit further down. > There are a couple of alternatives: > 1) Investigate why Gtk+ uses so many command lines and try to reduce it, > possibly by doing the * expansion inside g-ir-scanner instead of in cmd.exe A huge list of absolute (good IMHO) paths of the .c files to scan is part of the command line. Reducing this by placing GTK+'s source checkout in the shortest path possible (D:\g instead of D:\dev\gnome.org\checkout\gtk+) still leads to 12000+ characters... > 2) Use an alternative shell? All this developed and tested with bash. Well, MSYS bash which seems to inherit some of the limitations of cmd.exe or maybe more correctly the underlying API used by it. There is no other reliable shell available here that I know of... Note that no "terminal emulators" ala RXVT or mintty have been used, only the default "sh --login -i" from msys.bat > 3) Fix this solution properly, create a proper temporary file (where we can > write) and remove it afterwards. Will do, but maybe not remove it afterwards if GI_SCANNER_DEBUG="save-temps" (knowing what's in the arguments can be useful while debugging).
Created attachment 195963 [details] [review] [WIP] Windows port: honor XDG_DATA_DIRS, even on Windows On Monday, August 29 2011, jdahlin wrote: > I think he name of the function is wrong here. get_gir_dirs() perhaps? This was a quick hack. I thinks a more correct solution would be to fix GLib's g_system_data_dirs so it honors XDG_DATA_DIRS even on windows (it currently has a separate code path entirely for windows and thus leads to surprises like this...). But please correct me if I'm wrong ;)
Created attachment 195964 [details] [review] [WIP] Windows port: use the correct crt for Python 2.5, 2.6, 2.7 and 3.2 On Monday, August 29 2011, jdahlin wrote: > This could use some cleaning up. > if maj = 2: > if min = 5 -> 71 > else if min > 5 -> 90 > elif maj = 3 -> 90 I am working on deducing the crt used by the Python interpreter detected by ./configure somewhere else (python.m4) instead of this unmaintainable #if #elif nest but have not yet devised a reliable method to do so... Any thoughts, hints, ideas, ... are most welcome!
This series of patches enables gobject-introspection to be built on Windows with MinGW/MSYS. It also allows ATK, GDK-PixBuf, GTK+, Pango and PyGObject to be built on the same platform with introspection support. Things have evolved far enough for PyGObjct's gtk-demo to function mostly correctly. Well, there's some DnD releated crashes traced to GTK+ 3 that also happen with GTK+'s gtk-demo and a portability problem described below. So, here are the known issues so far: 1) Each module has to be built with a patched libtool (either the ltmain.sh or libtool script, depending on how the module uses libtool). The method used for this is already in use in Tor's build scripts and on OBS, either: sed -e 's/need_relink=yes/need_relink=no/' \ -e 's/wrappers_required=yes/wrappers_required=no/' \ <ltmain.sh >ltmain.temp && mv ltmain.temp ltmain.sh && or: sed -e 's/need_relink=yes/need_relink=no/' \ -e 's/wrappers_required=yes/wrappers_required=no/' \ <libtool >libtool.temp && mv libtool.temp libtool && 2) Tor's original patch (attachment #162747 [details]) contains a change described as "Accept also the __inline__ variant." and adds __inline__ right above __inline in giscanner/scannerlexer.l. I have tested both with and without this change and honestly don't see any difference... Does anybody know what this change is supposed to accomplish? 3) Some file related functions are missing from the generated .gir and .typelib files. The missing functions seem to always be related to file operations. So to get PyGObject's gtk-demo going, I'm currently forced to patch demos/gtk-demo/gtk-demo.py as follows: from gi.repository import GLib, GObject, Gio, Pango, GdkPixbuf, Gtk +GLib.file_test = GLib.file_test_utf8 +GLib.file_get_contents = GLib.file_get_contents_utf8 +GdkPixbuf.Pixbuf.new_from_file = GdkPixbuf.Pixbuf.new_from_file_utf8 +GdkPixbuf.Pixbuf.new_from_file_at_size = GdkPixbuf.Pixbuf.new_from_file_at_size_utf8 Note 1: On further investigation, it looks like these missing functions have been marked as PRIVATE in their respective .symbols files (see for example glib's glib/glib.symbols and search for g_file_test) and I suspect they are therefore ignored by giscanner. These .symbols files are used to generate .def files (export definition files), where the symbols in question are also marked as PRIVATE. The .def file on it's turn is then used to build the final .dll. Strange that for example analyzing libglib-2.0.dll with Dependency Walker, nm or objdump I can see g_file_test being exported right next to g_file_test_utf8. But maybe I just don't know where to look to learn how g_file_test is different from g_file_test_utf8 in libglib-2.0.dll... Note 2: Looks like this problem goes further than the .symbols files hackery described above. Took gdk-pixbuf as an test and removed the "DLL ABI stability hack" related bits. Now gi does what we'd expect. Yay. Only that breaking ABI for GLib, GDK-Pixbuf and maybe even GTK+ is probably going to be a bad idea...
Review of attachment 195955 [details] [review]: ::: giscanner/utils.py @@ +100,3 @@ # and pre-2.2. Johan 2008-10-21 libname = libname.replace('.libs/.libs', '.libs') + libname = libname.replace('.libs\\.libs', '.libs') libname = libname.replace(os.path.join(".libs", ".libs"), ".libs") should do the right thing. as os.path.join is kind of like path = path.replace(os.sep)
Review of attachment 195954 [details] [review]: ::: giscanner/shlibs.py @@ +70,3 @@ + if os.name == 'nt': + shlibs = [] What's the reason for avoiding libtool here? Will it cause any problems? Could this be related to the libtool problem that you mentioned in the README? I don't think Tor solved this part, but perhaps the code is newer than his patch.
(In reply to comment #13) > Review of attachment 195955 [details] [review]: > > ::: giscanner/utils.py > @@ +100,3 @@ > # and pre-2.2. Johan 2008-10-21 > libname = libname.replace('.libs/.libs', '.libs') > + libname = libname.replace('.libs\\.libs', '.libs') > > libname = libname.replace(os.path.join(".libs", ".libs"), ".libs") > > should do the right thing. as os.path.join is kind of like path = > path.replace(os.sep) Doing this doesn't work because of the 'Monkeypatch os.path.join' patch. The os.path.join patch ensure we don't have to sprinkle "os.path.join(..., ...).replace('\\', '/')" all over the place. So in this case doing what you propose would replace .libs/.libs with .libs but we explicitly also want .libs\.libs to be replaced with .libs It would have been easier if Python's ntpath.py (which is what you get for os.path on Windows) itself honored os.sep (so we could set os.sep to '/' for the MinGW/MSYS case) but it doesn't, so we're left with a single option: monkeypatching os.path.join().
(In reply to comment #14) > Review of attachment 195954 [details] [review]: > > ::: giscanner/shlibs.py > @@ +70,3 @@ > > + if os.name == 'nt': > + shlibs = [] > > What's the reason for avoiding libtool here? Will it cause any problems? > > Could this be related to the libtool problem that you mentioned in the README? > I don't think Tor solved this part, but perhaps the code is newer than his > patch. I'll have to revisit this one, don't remember all the details...
Review of attachment 195956 [details] [review]: ::: giscanner/dumper.py @@ +277,3 @@ else: + if os.name == 'nt': + lt_dll_pat = re.compile(r'^lib(.*)-[0-9][0-9]*$') Store this as a global variable, outside of this function. @@ +279,3 @@ + lt_dll_pat = re.compile(r'^lib(.*)-[0-9][0-9]*$') + if lt_dll_pat.match(library): + library = lt_dll_pat.sub(r'\1', library) As this is used in two places it should be moved to its own function
Review of attachment 195957 [details] [review]: Okay, this needs to be changed slightly. It should be #!/usr/bin/env pythonX.Y configure.ac should create a new environment variable which is the basename of the executable in PYTHON. PYTHON_BASENAME=`basename $PYTHON` AC_SUBST(PYTHON_BASENAME) or so, needs to be tested.
Review of attachment 195958 [details] [review]: Okay, I can't find a better way of doing this. So this can go in as it is.
Review of attachment 195959 [details] [review]: We should not monkey patch, instead we should always use say from giscanner.shutils import Popen In all places, and on Windows it would be a special subclass.
Review of attachment 195960 [details] [review]: This is wrong, we should use backslashes consistently on windows, it will help non-msys windows builds. For quoting there should be some utility function in subprocess.py or so.
Review of attachment 195961 [details] [review]: This seems wrong to me, it should be done in the same way on all platforms.
Review of attachment 195963 [details] [review]: ::: girepository/girparser.c @@ +259,3 @@ +static gchar **gi_system_data_dirs = NULL; + +const gchar * const * static @@ +260,3 @@ + +const gchar * const * +gi_get_system_data_dirs (void) get_data_dirs() or so perhaps? At least until glib is fixed and we can use that one
Review of attachment 195962 [details] [review]: ::: Makefile.introspection @@ +146,3 @@ + + $(_gir_silent_scanner_prefix) \ + PATH=.:.libs:$(PATH) \ This line is okay. The rest should go into another patch which I'm not sure how to do yet. I would hate if we'd have to use a temporary file, but let's see if there are any alternatives. @@ +160,3 @@ define introspection-compiler +$(_gir_silent_compiler) \ +PATH=.:.libs:$(PATH) \ This part is fine and can go in.
Created attachment 198149 [details] fix _resolve_non_libtool rationale: non_libtool case
Created attachment 198150 [details] fix _resolve_non_libtool rationale: libtool case In reply to comment #14 and comment #16: These attachments ("fix _resolve_non_libtool rationale: non_libtool case" and "fix _resolve_non_libtool rationale: libtool case") are written to: - be viewed side by side (a diff tool like meld would be ideal - explain what happens for the non_libtool case and the libtool case I didn't find a way to better explain this, so I hope it is enough. If not, please ask :)
I'm attempting to build glib 2.31.20 natively on Windows using mingw and msys. Regarding the comment above about using #!/usr/bin/env pythonX.Y, it's not working on my system. Mingw and msys don't come with an env program so this fails when I attempt to build in that environment. Perl is also being set up as /usr/bin/env perl which the system can't find. Am setting export PYTHON="/python/python" which is path and program name for Python. Both python and perl directories are in my PATH settings. Would be really nice if a user could set an environment variable or pass in the locations (and names of executables) for Python and Perl and the configure script would honor those names as is instead of replacing with #!/usr/bin/env constructs. If there's already a way to do it, couldn't find it in the documentation. Thanks.
(In reply to comment #27) > I'm attempting to build glib 2.31.20 natively on Windows using mingw and msys. > Regarding the comment above about using #!/usr/bin/env pythonX.Y, it's not > working on my system. Mingw and msys don't come with an env program so this > fails when I attempt to build in that environment. env.exe is part of the msys-coreutils package. If you find yourself without a working env program when inside an MSYS bash session, your MinGW/MSY installation is seriously broken. > Perl is also being set up > as /usr/bin/env perl which the system can't find. Am setting export > PYTHON="/python/python" which is path and program name for Python. You'd want to use MSYS Posix paths when you execute the export command from inside an MSYS bash session. Something like /c/Python27 or /c/bin/Python27. Python installation paths with spaces are discouraged on Windows, both by MingW and Python projects, so /c/Program\ Files/Python27 would be a bad idea. > Both python > and perl directories are in my PATH settings. If the example you've given above (PYTHON="/python/python") actually corresponds to how you've configured your system then they are not. > Would be really nice if a user > could set an environment variable or pass in the locations (and names of > executables) for Python and Perl and the configure script would honor those > names as is instead of replacing with #!/usr/bin/env constructs. If there's > already a way to do it, couldn't find it in the documentation. Thanks. When I release win32 binaries on www.gtk.org/ftp.gnome.org, do you want the scripts included with the binary zip package to have shebang lines to be hardcoded as #!/c/devel/opt/python27 ? Because not using #!/usr/bin/env constructs is going to give you exactly that. Without a way to fix it except for manually editing the script. mvg, Dieter
Just installed mingw and msys and development tools using their new install program a couple of weeks ago. If it's broken then something's wrong with the mingw install program. I haven't heard anyone else reporting issues with it being broken though. PATH=/usr/local/bin:/mingw/bin:/perl/bin:/bin:/usr/bin:/python etc/fstab includes: c:/perl /perl c:/python25 /python So, perl and python are in my paths. By "MSYS bash session", are you saying env should be available in a bash session with MSYSTEM set to MSYS or is it available in a normal bash session for native Windows development (MSYSTEM set to MINGW32)? As I said, just installed mingw and msys and development tools and env doesn't seem to be working, at least not the way it's being called from configure. Actually just the following in configure would fix the Python call on my end: if test "$PYTHON" = :; then PYTHON=$PYTHON else
Just tried building glib-2.31.22 instead of glib-2.31.20. (By the way, once I got python and perl locations adjusted in the configure on my end, glib-2.31.20 did build to completion on Windows.) When trying to build glib-2.31.22, I get the following error: glib-2.31.20\glib\tests/logging.c:264: undefined reference to `unsetenv' Checking the source, it seems unsetenv ("G_MESSAGES_DEBUG"); was just added to logging.c in version 2.31.22. It's not in the 2.31.20 version of logging.c.
I think I may have found the problem. Mingw installs env to \mingw\msys\bin. Within msys, that's equivalent to \bin. The glib configure file is looking for env in \usr\bin\ not \bin. My system follows the Linux File Hierarchy Standards (\bin does not by default equal \usr\bin). So, if you think following the Linux File Hierarchy Standards is seriously messed up, then you're right, my mingw installation is seriously messed up. I haven't tested yet, but putting a link from /bin/env to /usr/bin/env may fix the issue. Might be nice somewhere in the documentation to let other mingw users who run FHS compliant systems know that the glib configure scripts are looking for env in a place where it doesn't exist. I only posted the problem in the first place because your web page (http://developer.gnome.org/glib/2.30/glib-resources.html) encourages people to file bug reports. It says "Don't hesitate to file a bug report, even if you think we may know about it already, or aren't sure of the details." Maybe it should read, don't hesitate to file a bug report as long as you're not trying to build on Windows?
(In reply to comment #29) > etc/fstab includes: > c:/perl /perl > c:/python25 /python Sorry, but your install is broken as you have not executed /postinstall/pi.sh from an MSYS bash session. pi.sh would have added an entry to /etc/fstab similar to: C:/MinGW /mingw But please keep this bug report on topic: the gobject-instospection port to Windows. Questions about how to build GLib on Windows are most welcome on various mailing lists: https://mail.gnome.org/mailman/listinfo/gtk-list https://mail.gnome.org/mailman/listinfo/windows-devel-list and the #win32 channel on irc.gnome.org Thanks, Dieter
Apologies, I should have opened another bug report then, but I thought the issues were related since the title of this bug was "Win32 support" and the glib package does not build as in on Windows. I did execute /postinstall/pi.sh. Please don't attempt to guess what I did or did not do with my system. Your guesses so far are all inaccurate. You could simply ask. There is also no python25 on Windows when you install Python directly from their site ( http://www.python.org/getit/releases/2.5/ ). So, your configure script will not work as is. There is a python.exe and a pythonw.exe. One would need to add a link from phython.exe to python25 for your configure script to work and that is nowhere indicated in your documentation. I think I will look into qt and some of the other toolkits instead of gtk+, since your support seems to be very antagonistic to developers on certain platforms and you don't seem concerned at all about issues that keep your libraries from building on those platforms. I document information at many of the Windows development sites. Will be sure to recommend other GUI toolkits instead of gtk+ and to let other Windows developers know about the quality level for support they can expect to get if they do attempt to use gtk+ in their projects.
*** Bug 673915 has been marked as a duplicate of this bug. ***
Hi, (Sorry, as it did take a long time for me to get to anything here due to shortness of time :) ) I did manage to get the libraries, Python modules and the C-tools compiling under Visual C++, after purging some of the C99isms in the code, in particular those in cmph. But before I can post any patches for review, I get to the stage where we have to run g-ir-scanner, and that's where it stopped me, as it used cpp_args += ['-E', '-C', '-I.', '-'] (near line 278 or so in sourcescanner.py). Specifically, what it seems to me is that this command asked for running the preprocessor that accepts input from stdin (from what I could understand about the '-' argument), which is something that Visual C++ does not support AFAICT. So, any comments or pointers about this regard would be appreciated (or even "oh, this is a feature that is required for g-i" would do). So that's my progress for now. With blessings, thank you!
Hi, For references, and *not* intending to replace the efforts done in this bug, I have opened a new bug, bug 681820, for building g-i on Windows using Visual C++ With blessings, thank you!
Review of attachment 195958 [details] [review]: Hi, Can I know, to avoid any misunderstanding, whether it is okay for this patch to be committed (somehow it has status "accepted-commit now", but it's not yet in master)? Thank you, with blessings!
Presumably that patch depends on the other patches in this series which aren't yet commit-able.
(In reply to comment #37) > Review of attachment 195958 [details] [review]: > Can I know, to avoid any misunderstanding, whether it is okay for this patch to > be committed (somehow it has status "accepted-commit now", but it's not yet in > master)? Hi, Sorry for bugging here again... Just wondering, since the Visual Studio build files have already landed in 1.36/master, is it possible that patch 195958 be committed as that would also be useful with the Visual Studio builds? With blessings, thank you!
Review of attachment 195958 [details] [review]: Hi, As we have been getting support for building/using g-i for some time, I think it would be better for this patch to be pushed, especially when this patch was accepted without any further issues that were brought up about it. The patch was pushed as 7c1d8d28. With blessings, thank you!
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]
Created attachment 300028 [details] [review] Support passing arguments through a file (with a wrapper) Support passing arguments to g-ir-scanner through a file to avoid hitting the commandline length limit on W32. To ensure that no unnecessary mangling takes place, a special "--files-begin" argument is passed to the scanner, followed by the arguments that need to be put into a file. This way flags and other arguments that may not need to be mangled remain untouched (they likely *are* mangled by MSYS later on, but MSYS can and should have a better idea where, how and what should be mangled than we do). A wrapper script invocation is inserted into the scanner shebang (to do so, give G_IR_SCANNER_W32_WRAPPER="/mingw/lib/gobject-introspection/g-ir-scanner-w32-wrapper " (note the trailing space!) to `make'). The script looks for the "--files-begin" argument, removes it, and moves the rest of the arguments into a tempfile, appending --argfile=<name of the tempfile> to the argument list. After g-ir-scanner is done, the wrapper removes the tempfile and exits. Arguments moved into the file are DOSified. The scanner_main() function is modified to feed its arguments to a utility function that pre-processes the arguments, looking for the --argfile argument, and inserts in its place a shlex-parsed list from the tempfile. There is another _get_option_parser() invocation elswhere in scannermain.py, it remains unaltered. Because there's now a wrapper script in g-ir-scanner shebang, g-ir-scanner can't be run uninstalled. To work around that a g-ir-scanner-build is built with the script filename pointing to the source directory, and this version is used to make gir files during the build process of gobject-introspection.
Created attachment 300031 [details] [review] Support passing arguments through a file (with a wrapper) v2 Fixed the makefile (need to use a separate dirname and SCRIPTS).
Review of attachment 300031 [details] [review]: Please fix the nitpicks pointed inline ::: Makefile-giscanner.am @@ +132,3 @@ + +if OS_WIN32 +wrapperdir=$(pkglibdir) spaces ::: Makefile-tools.am @@ +15,3 @@ +TOOL_SUBSTITUTIONS_BUILD = sed -e s,@libdir\@,$(libdir), -e s,@datarootdir\@,$(datarootdir), -e s,@PYTHON\@,$(PYTHON), -e "s,@G_IR_SCANNER_W32_WRAPPER\@,$(abs_top_srcdir)/misc/g-ir-scanner-w32-wrapper ," +else +TOOL_SUBSTITUTIONS_BUILD = sed -e s,@libdir\@,$(libdir), -e s,@datarootdir\@,$(datarootdir), -e s,@PYTHON\@,$(PYTHON), -e s,@G_IR_SCANNER_W32_WRAPPER\@,, why the w32 wrapper in no win32? ::: Makefile.am @@ +55,3 @@ $(man_MANS) \ $(m4_DATA) \ + misc/g-ir-scanner-w32-wrapper \ fix the alignment ::: giscanner/scannermain.py @@ +471,2 @@ def scanner_main(args): + loaded_args = utils.maybe_load_args_from_file (args) no space before ( ::: giscanner/utils.py @@ +78,1 @@ +def load_args_from_file (filename): I think you should remove all the sapces before ( ::: misc/g-ir-scanner-w32-wrapper @@ +2,3 @@ +# This script is a w32 wrapper for g-ir-scanner, +# it will put all arguments after "--files-begin" into a tempfile +# and replace them with --argfile <tempfile> before invoking g-ir-scanner empty line @@ +5,3 @@ +new_args= +while (( "$#" )); do + if test "x$1" == "x--files-begin" check if other scripts use 2 spaces for indentation or something else @@ +13,3 @@ + fi + shift +done leave empty line @@ +20,3 @@ + dosedx=$(cmd //C echo $x) + echo -n "$dosedx " >>$argfile +done leave empty line @@ +24,3 @@ +result=$? +rm -f $argfile +exit $result empty line
Created attachment 300090 [details] [review] Support passing arguments through a file (with a wrapper) v3 Fixed nitpicks
Created attachment 300091 [details] [review] Support passing arguments through a file (with a wrapper) v4 Also changed the Python parts indentation to 4 spaces.
Created attachment 300239 [details] [review] Support passing arguments through a file (with a wrapper) v5 Add \" \" escaped doublequotes around $1 when forming the new argument list (otherwise arguments with spaces, like --libtool="/usr/bin/sh ./libtool" will fall apart).
Created attachment 300240 [details] [review] Support passing arguments through a file (with a wrapper) v6 Check that an argument contains a space, only surround it with double quotes if it does. If this isn't done, i get an error like this: /mingw/lib/gobject-introspection/g-ir-scanner-w32-wrapper: line 35: "/mingw/bin/python2.7": No such file or directory (which is bogus, there is such file; it's just that literal double-quotes in the first argument are interpreted literally) At this point i'm ready to concede that maybe using a wrapper shell script is not a good idea (OTOH, maybe this is the last hurdle we're going to see; it's not like this script must handle *anything* one could write in shell, just anything that could be conceivably fed to g-ir-scanner from a makefile...).
We could try to use a Python script as a wrapper (using the MSYS/Cygwin Python, which has no argument length limit). It will likely have no weird quoting problems, and should be able to do all the necessary transmutations just as easily as the shell script (if with a bit more verbosity, with subprocess.Popen and all). Or MSYS/Cygwin Perl. But in that case i can't help, i can barely *read* Perl, let alone write it.
OK, i give up. I've tried to rewrite the wrapper in Python, and realized that either way what we need is to produce a partial commandline in the file that can be parsed by shlex. That's just non-trivial. I'll try to re-work this in the style of the previous patch, the one that did conversion in g-i.
Created attachment 300403 [details] [review] Support passing arguments through a file (with a wrapper) v7 I didn't give up. Re-wrote the wrapper in Python. This gets rid of argument parsing and escaping problems. I've hardcoded /usr/bin/python as the interpretor, but i'm open to suggestions. It must be an MSYS/Cygwin Python through, obviously. Argfile is now written in a binary format (UTF-8 text + NUL-separators). This gets rid of uncertainty over the way file contents will be parsed. Argfile is now parsed by Python code in g-i, not by shlex. Because see above. --files-begin is now only inserted on W32. On other platforms it's not used. Accordingly, g-i will only accept --argfile=... argument on W32. This should clear problems with other platforms, where embedded NULs are legal in filenames.
Review of attachment 300403 [details] [review]: Initial review. ::: Makefile-giscanner.am @@ +133,3 @@ +if OS_WIN32 +wrapperdir = $(pkglibdir) +wrapper_SCRIPTS = misc/g-ir-scanner-w32-wrapper do we need to extra dist it? ::: Makefile.am @@ +55,3 @@ + $(man_MANS) \ + $(m4_DATA) \ + misc/g-ir-scanner-w32-wrapper \ mmm so you added the extra dist here, I wonder if it should be in the other makefile ::: giscanner/utils.py @@ +85,3 @@ + a_file.append(a_byte) + else: + files.append(b''.join(a_file).decode ('UTF-8')) remove space before ( @@ +91,3 @@ +def maybe_load_args_from_file(args): + if os.name != 'nt': + return args I'd say empty line after the return @@ +103,3 @@ + skip = True + processed_args.append(arg) + continue empty line between different ifs @@ +107,3 @@ + processed_args.extend(load_args_from_file(arg)) + is_input_file = False + continue same here ::: misc/g-ir-scanner-w32-wrapper @@ +1,2 @@ +#!/usr/bin/python +# This script is a w32 wrapper for g-ir-scanner, add the copyright stuff
(In reply to Ignacio Casal Quinteiro (nacho) from comment #52) > Review of attachment 300403 [details] [review] [review]: > > Initial review. > > ::: Makefile-giscanner.am > @@ +133,3 @@ > +if OS_WIN32 > +wrapperdir = $(pkglibdir) > +wrapper_SCRIPTS = misc/g-ir-scanner-w32-wrapper > > do we need to extra dist it? No idea. > > ::: Makefile.am > @@ +55,3 @@ > + $(man_MANS) \ > + $(m4_DATA) \ > + misc/g-ir-scanner-w32-wrapper \ > > mmm so you added the extra dist here, I wonder if it should be in the other > makefile No idea. But then, the Makefile-tools.am only refers to tools/* in EXTRA_DIST, so for the sake of consistency it maybe should be in Makefile.am OTOH, i'm not sure it needs to be in EXTRA_DIST at all. I guess there's only way to find out - do an `make dist' and see the result.
Created attachment 300435 [details] [review] Support passing arguments through a file (with a wrapper) v7.1 Fixed nitpicks. Tried removing the wrapper script for EXTRA_DIST, this caused it to be absent in the tarball produced by `make dist-xz', so i'm going to keep it. I can move it to Makefile-tools.am, if that's needed.
Comment on attachment 195955 [details] [review] replace a stray backslash with a slash. current giscanner/utils.py already has this
Created attachment 300510 [details] [review] Support passing arguments through a file (with a wrapper) v8 Rolled back the change in dist_make_DATA (it should not have an .in there). Spoke with Alexey about this patch, and he was somewhat unhappy about the use of /usr/bin/python as a wrapper (claiming that this introduces a dependency that shouldn't be there). Essentially, the problem is that the argument list formed by the makefile rule for g-ir-scanner invocation is too long. One way of fixing this is to feed the arguments to something msys-sy (thus having no commandline length limit), which would then trim the commandline, put the rest of the args into a file and use the --argfile trick. This is what i did. Another way is to coerce `make' into doing the same thing (at least partially). I've tried doing it the easy way by adding extra shell script commands to the g-ir-scanner invocation in Makefile.introspection. This does not work, probably because is dynamically generated by the `introspection-scanner' macro, which prevents any $variable expansions that i know of. Now, we could just forgo looping and instead just echo the whole $$^ prerequisite list into a file named $(1).files (there was never really a need to use a uniquely-named tempfile, $(1) ensures no conflicts) and supply an --argfile=$(1).files to the scanner - i.e. the way the original patch did. This will fix the immediate argument-list-is-too-long problem. There are two downsides: * cygpath (or equivalent) have to be called by g-ir-scanner, not encapsulated in the wrapper. Not a big problem, scanner already does a lot of very platform-specific calls for various purposes. * $(1).files will (again) be parsed by the shlex module - we can't add \0-separators or even quote individual names, we get what "echo $$^" gives us. This may be a problem if there are filenames with spaces. The mitigating factor is that we (supposedly) *know* that $$^ only contains filenames and not any arguments of other kind, such as the problematic --libtool="/bin/sh ./libtool" (which requires quoting). As long as filenames are space-free, it should be OK.
Created attachment 300561 [details] [review] Support passing arguments through a file (with a makefile rule) v1 That idea panned out. This is mostly a step back to the original patches (attachment 195961 [details] [review] and attachment 195962 [details] [review]), with the following differences: * Makefile.introspection is preprocessed, --argfile is only used on W32 * --argfile only contains the files from $$^ instead of the whole commandline. This way flags and options won't be messed up (filenames are, hopefully, space-free, and thus safe to pass through a file). * argfile contents are parsed by shlex * code for reading argfile is specific to W32 * argfile processing is done before main option parser is invoked, results are more predictable
Created attachment 300953 [details] [review] Support passing arguments through a file (with a makefile rule) v1.1 * Fixed code style " (" -> "(" * Added missing comma in the list of subprocess.Popen() arguments * Fixed condition (!= nt -> == nt)
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gobject-introspection/issues/27.