RE: [PATCH] Refactor and merge child_execute_job() code, new attempt
Hello! I like this patch; it looks like a good improvement! It works fine for me on UNIX systems. Only one comment: - /* undo CLOSE_ON_EXEC() after the child process has been started */ + /* undo FD_CLOEXEC after the child process has been started */ I think this change should be reverted, since we decided to keep the CLOSE_ON_EXEC() macro. Of course, i've just forgot about the comment. :) Ok, i'm waiting for the change to be committed in order to proceed with Cygwin specifics. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH] Refactor and merge child_execute_job() code, new attempt
Hello, Paul! Sorry for so long delay, i'm really quite busy, however i have found some time to get back to this. Please review the new version. This is actually a repost. I have posted this message a week ago and got no response. I suggest it fell into old thread, so you have missed it. Monday, February 3, 2014, 0:30:26 you wrote: I prefer the macro form as well; please keep it that way. Done. Also, I'm not sure I like the current child_execute_job() where there are two completely different implementations in the same function, handled by ifdef. But I guess we do the same thing in other places. Yes. This function does very OS-specific things, and personally i don't see how it could be done in some other way. Well, except if we completely split it up and introduce some new OS-specific .c files which will hold implementations of OS-specific functions. Can you push down the environ pointer resetting into exec_command(), rather than having the save/restore in start_job_command()? We don't need this on POSIX systems so it would be nice if it was not done there. Done. After some code review i have noticed that we actually don't need this at all when using fork(), because execvp() is executed in child context. So only spawn() code actually uses it now. However it seems we might be able to simplify some things, especially on POSIX systems, by pushing the handling of it down into child_execute_job(). To do that we'd need to pass the flags argument to child_execute_job(), or at least a boolean value specifying whether COMMANDS_RECURSE is set. But then on POSIX systems, at least, we could do the close-on-exec in the child's fork and we wouldn't have to undo it. Yes, we can do also this. However perhaps this would not look too good because we call child_execute_job() from two different places, and we need to deal with these fd's only in one place. I think it's not a big loss to reset the flag. -- С уважением, Pavel mailto:pavel_fe...@mail.ru make-merge-child_execute_job-code.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH] Refactor and merge child_execute_job() code
Hello, Paul! Sorry for so long delay, i'm really quite busy, however i have found some time to get back to this. Please review the new version. Monday, February 3, 2014, 0:30:26 you wrote: I prefer the macro form as well; please keep it that way. Done. Also, I'm not sure I like the current child_execute_job() where there are two completely different implementations in the same function, handled by ifdef. But I guess we do the same thing in other places. Yes. This function does very OS-specific things, and personally i don't see how it could be done in some other way. Well, except if we completely split it up and introduce some new OS-specific .c files which will hold implementations of OS-specific functions. Can you push down the environ pointer resetting into exec_command(), rather than having the save/restore in start_job_command()? We don't need this on POSIX systems so it would be nice if it was not done there. Done. After some code review i have noticed that we actually don't need this at all when using fork(), because execvp() is executed in child context. So only spawn() code actually uses it now. However it seems we might be able to simplify some things, especially on POSIX systems, by pushing the handling of it down into child_execute_job(). To do that we'd need to pass the flags argument to child_execute_job(), or at least a boolean value specifying whether COMMANDS_RECURSE is set. But then on POSIX systems, at least, we could do the close-on-exec in the child's fork and we wouldn't have to undo it. Yes, we can do also this. However perhaps this would not look too good because we call child_execute_job() from two different places, and we need to deal with these fd's only in one place. I think it's not a big loss to reset the flag. -- С уважением, Pavel mailto:pavel_fe...@mail.ru make-merge-child_execute_job-code.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH] Refactor and merge child_execute_job() code
Hello, Paul! I have read your suggestions and i think i can implement them. But, sorry, not earlier than weekend. Starting to be extremely busy... -- С уважением, Pavel mailto:pavel_fe...@mail.ru ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH] Refactor and merge child_execute_job() code
Hello! This is my long-promised refactor. After this it's much easier to apply runtime selection between spawn() and fork() on Cygwin, because all differences are now consolidated in two functions: child_execute_process() and exec_command(). Insome critical POSIX-specific places i have expanded CLOSE_ON_EXEC() macro in order to provide reliable build problem tracking. As far as i understand, even very old POSIX systems implement this flag, so this should be OK. I've done it in order to prevent theoretical problems with e. g. forgotten #include somewhere, or some problems with headers. Old code in this case would silently define CLOSE_ON_EXEC() to empty. Test results: --- cut --- $ ./run_make_tests.pl -make /usr/src/make/make.exe -- Running tests for GNU make on CYGWIN_NT-5.1 netbook 1.7.27(0.271/5/3) i686 GNU Make 4.0.90 -- Clearing work... Finding tests... features/archives ... ok (10 passed) features/comments ... ok (1 passed) features/conditionals ... ok (4 passed) features/default_names .. ok (2 passed) features/double_colon ... ok (11 passed) features/echoing ok (4 passed) features/errors . ok (2 passed) features/escape . ok (8 passed) features/export . ok (12 passed) features/include ok (10 passed) features/jobserver .. ok (2 passed) features/load ... N/A features/loadapi N/A features/mult_rules . ok (2 passed) features/mult_targets ... ok (2 passed) features/order_only . ok (10 passed) features/output-sync ok (14 passed) features/override ... ok (4 passed) features/parallelism ok (9 passed) features/patspecific_vars ... ok (10 passed) features/patternrules ... ok (10 passed) features/quoting ok (1 passed) features/recursion .. ok (2 passed) features/reinvoke ... ok (5 passed) features/rule_glob .. ok (3 passed) features/se_explicit ok (10 passed) features/se_implicit ok (11 passed) features/se_statpat . ok (4 passed) features/shell_assignment ... ok (4 passed) features/statipattrules . ok (8 passed) features/targetvars . ok (25 passed) features/utf8 ... ok (1 passed) features/varnesting . ok (2 passed) features/vpath .. ok (2 passed) features/vpath2 . ok (1 passed) features/vpath3 . ok (1 passed) features/vpathgpath . ok (1 passed) features/vpathplus .. ok (4 passed) functions/abspath ... ok (1 passed) functions/addprefix . ok (1 passed) functions/addsuffix . ok (2 passed) functions/andor . ok (2 passed) functions/basename .. ok (1 passed) functions/call .. ok (3 passed) functions/dir ... ok (1 passed) functions/error . ok (5 passed) functions/eval .. ok (9 passed) functions/file .. *** Test died (functions/file): Opened read-only file! FAILED (2/2 passed) functions/filter-out ok (5 passed) functions/findstring ok (1 passed) functions/flavor
[PATCH] Refactor and merge child_execute_job() code - small update
Ooops, sorry. It's really too late here, so i'm a bit impatient. Forgot to regenerate .diff after last-minute change. Better use this version. In it return type of child_execute_job() is changed from int to pid_t, as it should be. -- С уважением, Pavel mailto:pavel_fe...@mail.ru make-merge-child_execute_job-code.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH] Fix output-sync option on EMX - updated
Hello! Done. Thank you very much! My take on this is that I need to see the refactoring to say something intelligent about it. Ok, of course. :) Currently i have some high-priority problems here, when i have more time i'll get back to it and of course post the patch. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH] Fix output-sync option on EMX - updated
Hello! Thanks, this variant is fine with me. Good. I'm waiting for it to be committed, since i would like to go on with merging spawn-patch. In order to make it switched at runtime, i would like to refactor child_execute_job(). I have suggested this earlier, nobody replied. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH] Fix output-sync option on EMX - updated
Hello!
Thanks, but does EMX support output-sync? If not, this fragment:
+ /* Divert child output if output_sync in use. */
+ if (child-output.syncout)
+{
+ if (child-output.out = 0)
+outfd = child-output.out;
+ if (child-output.err = 0)
+errfd = child-output.err;
+}
+
should be conditioned on NO_OUTPUT_SYNC. Otherwise we are breaking the
EMX build of Make.
I have rechecked. Actually NO_OUTPUT_SYNC is defined only by handmade
config.h files for DOS, VMS and Amiga. EMX uses configure script, so
NO_OUTPUT_SYNC will not be defined.
However, indeed, there is rational seed in what you say. Since this code
actually goes to generic part, it will be useful to be able to disable this
code, just in case.
Please take the updated patch. I have verified it to build by supplying
CPPFLAGS=-DNO_OUTPUT_SYNC to configure.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
make-fix-output-sync-emx.diff
Description: Binary data
___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
Re: win32 compilation of make 4.0 source code
Title: Re: win32 compilation of make 4.0 source code Hello, Mark. Monday, January 13, 2014, 17:57:20 you wrote: Is the resulting file supposed to executed under Cygwin ? No. If you build Make natively for Windows, this results in native Windows version. Cygwin is actually quite a different story. Cygwin is not really Windows. Perhaps you heard the word "personalities". Windows NT in the beginning was supposed to have multiple "personalities" (read: APIs) to be compatible (sometimes even on binary level) with different OSes. These personalities included WinAPI (actually old 9x API), OS/2 (binary-compatible) and UNIX (only source-compatible because still relied on PE format, not ELF). With time Microsoft decided to phase our everything except WinAPI. Cygwin is actually modern, opensource version of UNIX personality. When you build a program for Cygwin, you use Cygwin compilers, not Visual Studio or MinGW. Cygwin appears to be a different platform with a different runtime. You cannot compile for Cygwin using Visual Studio (well, in theory you can, given you supply a set of proper includes and libraries), but i don't know about practical attempts to do that. -- , Pavel mailto:pavel_fe...@mail.ru ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Missing po files in GIT
Hello! I am trying to rebuild GIT version of Make, however .po files are missing in the repository. Is this intentional ? I have copied them over from my 4.0-2 archive. But where are they originally stored ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH] Fix output-sync option on EMX
Hello! This is part of my spawn-patch for Make. The purpose of this piece is to add missing support for output-sync option to spawn()-based flavors (currently only EMX). Tested together with the rest of spawn-patch under Cygwin, works fine. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia make-fix-output-sync-emx.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: win32 compilation of make 4.0 source code
=== process_begin: CreateProcess(NULL, uname, ...) failed. make: process_begin: CreateProcess(NULL, uname -a, ...) failed. make: process_begin: CreateProcess(NULL, cygpath C:\zzz_13.12.1_gener make: process_begin: CreateProcess(NULL, pwd, ...) failed. make: process_begin: CreateProcess(NULL, basename , ...) failed. make: process_begin: CreateProcess(NULL, pwd, ...) failed. make: process_begin: CreateProcess(NULL, basename , ...) failed. make: '.' is not recognized as an internal or external command, operable program or batch file. ... = I have looked at the output carefully... Obviously, you are trying to execute Makefile written for UNIX systems, are you ? This makefile relies on UNIX shell commands like uname, pwd, basename, etc. Right ? In order to run this Makefile, you need Cygwin or MinGW32 environment. Native Windows version of Make executes commands via cmd.exe, which of course does not understand commonly used UNIX stuff. Well, some commands are emulated by Make itself in such a case, but only a very small number. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[BUG] GNU Make - bad default .LIBPATTERNS on Windows/Cygwin
Hello! I've just discovered one more bug in GNU Make under Cygwin. Make is able to understand -lfoo as depencencies and tries its best to look up libraries correctly. However in some situations it fails to do so because it lacks proper templates for Cygwin. The problem is fixed by adding the following proper definitions to default.c: --- cut --- #elif defined(__CYGWIN__) || defined(WINDOWS32) .LIBPATTERNS, lib%.dll.a %.dll.a lib%.a %.lib lib%.dll %.dll, --- cut --- I have obtained these rules from GNU ld source code: http://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=ld/emultemp l/pe.em;h=5d6da9e05f14425cfad9cc4ec71e1a20e450b336;hb=HEAD#l2068 Since this file is for all PE targets, i suggest that the same applies also to native Windows target. The problem was triggered by attempting to rebuild nfs-server package with tirpc library. Its makefiles list $(LIBS) as dependencies. tirpc package for x86-64 for some reason does not include static version libtirpc.a, only libtirpc.dll.a. This caused the failure. I decided to cross-post this also in Cygwin ML for convenience. By the way... A good question: is this approach correct ? Actually i could cross-compile this package for e.g. Linux host, and in this case i would have come to the inversed version of the same problem. May be Make could be more smart and somehow know for which target we are compiling ? For example, this could be done by calling $(CC) -dumpmachine (in case of gcc = v4). Or could it check something like $(HOST) set by makefile ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: Issue with building make with mingw32 compilers.
Hello, Eli. Saturday, November 23, 2013, 12:00:49 you wrote: and that means that I need to use --host=i586-mingw32msvc when building make. Why does the prefix end with msvc? The MSVC compiler is not involved in this build or in the target system, so the prefix sounds wrong. May be simply replace mingw32 with mingw ? There is also mingw64, whose crosscompilers will have something like mingw64-w64 prefix. Don't ask me why, ask maintainers... -- С уважением, Pavel mailto:pavel_fe...@mail.ru ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
VMS port
Hello! I am restarting work on spawn-patch for Cygwin. Actually, i have the very first version working, but want to try to do some face-lift and get rid of some #ifdef's. My first question is: is VMS port maintained, or dead long ago ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: VMS port
Hello!
The VMS port is actively and capably maintained by Hartmut Becker. The
ChangeLog shows he provided VMS fixes for 4.0 as recently as September.
Ah, i see. Thanks for pointing at.
It's easier if patches are targeted for specific results, so it's best
not to include major refactorings, like removing ports, in the same
patch.
I know, this is what i'm going to do. Actually, i want to fix output-sync for
spawn()-based flavor. This includes EMX, DOS and potentially Cygwin. Currently
output-sync option will not work in that ports, because the related fragment:
--- cut ---
/* Divert child output if output_sync in use. */
if (child-output.syncout)
{
if (child-output.out = 0)
outfd = child-output.out;
if (child-output.err = 0)
errfd = child-output.err;
}
--- cut ---
is for some reason under #else branch only (where fork() is used). I think
that the best way to fix this is to move this fragment out of #ifdef's. But
above there's one more #ifdef VMS fragment, which also seems to ignore
child-output settings. I see that VMS version of 'child_execute_job' gets the
complete 'child' structure, i have checked it but it seems to ignore these new
members.
So i've got one more question - may be this fragment should become completely
generic ? Just with some note that VMS code should also implement handling for
it.
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
Some more refactor suggestions
Hello! I would like to suggest another code refactor, which could make the code easier to maintain. This is a result of my study of spawn() vs fork() differences and ways to implement runtime switching between spawn() and fork() in a simple way. The idea is to unify child_execute_job() function. The unification can be done in the following way: 1. The function will always return child's PID. 2. On UNIX the function will call fork() in order to create a child by itself. 3. job_fds[] and job_rfd can be handled in spawn() way in both cases. Currently the differences are: a) fork() version calls fork(), then closes descriptors in the child explicitly. b) spawn() version sets FD_CLOEXEC flags for these fd's, then calls spawn(). This effectively means that these descriptors are not inherited by the child. After spawn()ing FD_CLOEXEC flag is reset. I suggest that (b) approach can also be used on UNIX without bad effects. We could FD_CLONEXEC for relevand fds, then call child_execute_job() which does fork(). The effect should be the same as closing fd's in the child manually. I suggest that parent process can even skip resetting this flag since Make AFAIK never replaces itself with some other process which needs to inherit all internal file descriptors. So, the code can be further simplified. Any thoughts / pros / contras ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: make check under Cygwin
Hello! Please take a look at the change I made, it takes care of all those things at a price of only 5 changed lines. I see. Should work, indeed. I'd say the resulting code is less optimal (repeated conditions, strncpy() with length = 1) and a bit more difficult to understand. But, after all, it's a matter of personal taste, so feel free to ignore it. ;) One more small note. Since EMX is an environment similar to Cygwin in purpose (according to the description), i suggest that it has the same property and should also understand both forms of path. So, ok, i promise to take a look at my spawn patch on weekend. I remember about env-variable agreement. :) Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: make check under Cygwin (3.99.93)
Hello! Also, for information with the spawn-patch, the output-sync test still fails (like for 3.99.92, this is not unexpected), but now (with 3.99.93) the GNUMAKEFLAGS test also fails. Thank you very much for testing. I will really try to apply my hands to it this weekend. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: make check under Cygwin
Hello! I think the problem is abspath, which fails on Cygwin with DOS-style file names with a drive letter. Fixing that function on Cygwin is a priority for this release, if possible. The patch you suggest, OTOH, entirely disables support for DOS-style file names in the Cygwin build, which is a much worse alternative. Just a reminder. I have followed your suggestion and fixed this a month ago. Please try this: http://lists.gnu.org/archive/html/bug-make/2013-08/msg00031.html Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH] Use spawn() in GNU Make on Cygwin, updated
I really consider the use of Cygwin's spawn() deprecated and I'm not really interested in spending time on it. Why ? It is a way to significantly increase performance. And, before Cygwin has posix_spawn(), it is the only way to do it. I was following various fork() discussions, Microsoft is not interested in solving this because they say absolutely the same phrase about POSIX and all that. And, in fact, their phrase weights much more, because they already own the computing world. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH] Use spawn() in GNU Make on Cygwin, updated
Hello! Sorry for silence, i was completely busy with other interesting stuff... By this time i have calmed down and remade my thoughts... First, since i am interested, i will conduct the research about that code between fork() and exec() and try to implement some test cases for it. Perhaps this can have also something to do with #39825. Alternative idea. Looks like you don't like spawn() because it is poorly documented and isn't guaranteed to behave the same on all operating systems. What if we implement posix_spawn() for Cygwin ? Would you like that ? One more alternative idea. What if we clearly define spawn() specification for Cygwin ? It can be based on the rationale that spawn() is there to offer more performant version of fork() + exec(), and its behavior, ideally, should strictly follow original semantics of this pair. Eli, Christopher ? Your responses to this ? P. S. Eli, what about my other work (abspath fix) ? I don't see it in GIT log, and also i see no feedback, so... May be you just don't want to accept anything, and i'm just annoying you ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH] Use spawn() in GNU Make on Cygwin, updated
Hello, Paul. Friday, August 16, 2013, 22:18:31 you wrote: Presumably make works at least 99% correctly on Windows using spawn*(). I don't doubt at all that the patch actually works great with most uses of make in Cygwin. However, I would rather be 100% correct and slower than 99% correct with head scratching corner case errors. Exactly, hence the reason for my question. I'm not interested in adding this if, when it's enabled, things don't work correctly. On the other hand I'm not sure it's not possible to get things working correctly. Or, perhaps it's possible to make them work correctly Let me come in again ? Let's take a look at what we have. We have a pending change, which has undergone some testing. It works. At the other hand, there can be some corner cases. May be let's be constructive and test them, instead of just speculating about what can happen and what cannot ? A small analogy: you can speculate that tomorrow, when you come out of your home, a brick can fell from the roof and kill you. Now what ? Don't ever come out of house ? Wear builder's helmet ? :) To me current situation looks non-constructive. You say: Current implementation works, new implementation theoretically may fail (because it's new), so we must not change the code. Is it correct developer's approach ? Yes, changes sometimes introduce bugs, that's OK. Doesn't this mean that we just need to add some tests ? Can anyone (opponents) give ideas on what exactly needs to be tested ? I can extend make's test suite then. -- С уважением, Pavel mailto:pavel_fe...@mail.ru ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH 1/2] Fix abspath on Cygwin/MSYS with DOS paths enabled
Hello! This is the new patch. Allows to enable DOS paths on Cygwin with no abspath breaking. The idea of the change: instead of single HAVE_DOS_PATHS definition we now have two definitions: 1. HAVE_DOS_PATHS - says that the platform is capable of handling DOS paths in principle. 2. NATIVE_DOS_PATHS - says that DOS paths are native for this platform, and functions like getcwd() return DOS-style paths. NATIVE_DOS_PATHS now determines two things: 1. Expansion of relative paths (root_len value). 2. Fallback value of PATH_SEPARATOR_CHAR, unless provided by configure. I have tested the change on Cygwin and Windows. Works fine, no problems were discovered. Unfortunately test suite reports lots of errors for Windows version because it's designed to work only on POSIX, but visual inspection of results reveals that abspath works correctly in both cases. Now it should be OK to keep HAVE_DOS_PATHS enabled on Cygwin, with this change configure hint can be removed. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia make-3.82.90-1-fix-dos-paths-compatibility.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH 2/2] Do not use DOS paths on Cygwin
Hello! This is what I'd expect. Good then. 2. PATH_SEPARATOR on Cygwin is ':' and on pure DOS/Windows is ';'. This is true, but how is this relevant to the issue at hand? 'abspath' does not deal with PATH-style directory lists, it accepts a single file name as its argument. What am I missing here? To the issue - no, this really doesn't relate. But this is still involved in the change, because path list is one more thing which depends on native path style of underlying environment. It probably would, if you do a clean job, but frankly I'm not sure it's worth your while. It is so easy to glean this information by just looking at the appropriate preprocessor macro, like WINDOWS32 etc. that a configure-time test sounds like overkill. Yes, but there can be one thing, considering MinGW. If you don't know, MinGW actually consists of two environments: MinGW by itself and MSYS. MinGW by itself is a pure Windows environment. MSYS is enlightened version of Cygwin whose purpose is to provide enough compatibility to run UNIX shell scripts like configure. MinGW land uses DOS-style paths and MSYS land uses UNIX-style ones. Consequently, they actually build two versions of make. One version is called 'mingw-make' and is a pure Windows program. Another version is called 'make' and operates in MSYS land, dealing with UNIX-style paths and path lists. They do this because majority of build systems use POSIX semantics. Both versions are built from the same source. Consequently, there should be a possibility to override native path format test, similar to what Cygwin currently does to disable DOS paths. This can be needed for building MSYS version. Ok, good, currently i already have an implementation which works fine on Cygwin and passes all test suite. Now i want to try to build pure Windows version and make sure that it at least doesn't become worse. I need some time to test it and to approve the patch for submission here at Samsung. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH] Use spawn() in GNU Make on Cygwin, updated
Hello! I tried to explain that in my first response: 'fork' has a certain semantics and implements requirements that 'spawn' does not. Stop stop stop... Just to avoid misunderstanding here... fork() alone cannot be replaced with spawn(), yes. But as fas as i understand, make does not use fork() alone. It uses fork() + exec() pair in order to start external programs. As an end result, it is expected to run an external executable with some defined environment (make sets environment in the child then uses execvp() where external binary inherits it ). spawnvpe() does absolutely the same, but in one shot. It runs an external executable with predefined environment. Similar to fork(), spawnvpe() returns child's PID, which allows to wait for its termination, send signals, etc. And of course the started binary runs completely asynchronously, the same as with fork() + exec(). Windows actually does not allow to fork() cleanly by design (there's a full explanation in http://www.cygwin.com/cygwin-ug-net/highlights.html ). spawn() is much faster because the child is created from scratch with a new executable, it does not have to be a clone of the parent, so it perfectly wraps to legitimate CreateProcess() call. $(shell ) function also works fine, consequently we really perfectly handle stdin/stdout (without this i don't see how command output would be captured). So... I'm not sure (Chris?). In any case, you will see in the code that Make does a few things between the calls to 'fork' and 'exec'. How can you be sure that 'spawn' does all that in exactly the same way? EMX/Cygwin code does absolutely the same things before calling spawn(). So everything is set up. Yes, this means that this setup happens in parent too. But - after all everything works fine. You have got feedbacks, modified make works. Yes, there was a small problem discovered, but i have fixed it. And even if does that today, how can anyone be sure the Cygwin maintainers will not change it one day to do things in a slightly different way? Well, i hardly believe they can just change spawn() to something incompatible. Despite it's not POSIX function, it actually has a specification. It can be found here http://msdn.microsoft.com/en-us/library/20y988d2(v=vs.80).aspx or here http://www.qnx.com/developers/docs/6.3.0SP3/neutrino/lib_ref/s/spawnvpe.html . And, amongst other things, the specification clearly says that with certain flags (P_OVERLAY) it's a complete equivalent to exec(). So, consequently, all POSIX behavior which applies to exec(), automatically applies to spawn(), it technically can't be the other way. After all, if some developer changes spawn() so that something stops working, doesn't this mean that he has just introduced a bug and should fix it ? Well, if you really want it, changing child's environment can be #ifdef'ed. Yes, please. Ok, this is not a problem and in my next attempt i'll add this. You mean, rename 'status to 'child_status'? That's not a problem. Yes, please. If we don't come to full agreement, let's move in parts. Actually i understand that you just want to make sure that the program doesn't break. Perhaps some more testing would convince you. Actually i have sent the new patch to Denis Excoffier (who reported the bug), but he doesn't reply for some reason. If there'll be no response for some more time, i'll retry posting the patch on Cygwin ML. I just don't want to be too much annoying. I hope you also understand me right, i just want to avoid unnecessary overcomplications. I think the change should be simple. The simpler the modification, the more chances it's not broken somehow. This is the reason why i reuse EMX code for Cygwin - since it's already there, i assume it's already tested and works. By the way, who told that EMX is not POSIX ? Actually it is POSIX, it is similar to Cygwin (it provides POSIX environment on top of non-POSIX one). See http://en.wikipedia.org/wiki/EMX_(programming_environment) . I believe it just misses fork() because of some underlying OS limitations. But IMHO this is still POSIX. For example, there is a similar environment on AmigaOS (and derivatives) called ixemul, it is also a full POSIX API, but also cannot do fork() because the operating system does not work with address spaces (it's similar to ucLinux or ELKS). So, being unable to fork() does not automatically mean being non-POSIX. But, well, this is just an ideological argue and can be omitted. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH 2/2] Do not use DOS paths on Cygwin
Hello! Are you saying that if a Cygwin Make uses DOS style file names with drive letters, it also expects the PATH-style directory lists to use a semi-colon as a separator? I don't think I'd expect that. There is a default in make.h: --- cut --- /* Handle other OSs. */ #ifndef PATH_SEPARATOR_CHAR # if defined(HAVE_DOS_PATHS) # define PATH_SEPARATOR_CHAR ';' # elif defined(VMS) # define PATH_SEPARATOR_CHAR ',' # else # define PATH_SEPARATOR_CHAR ':' # endif #endif --- cut --- But, i have just checked, it gets overridden by configure script. So if you configure on Cygwin, it will be correct. But, well, i was talking about this default actually. 2 Cristopher: but it never allows both, this is simply impossible by design. I think PATH-style lists should always use Unix-style file named in the Cygwin Make. DOS-style file names should only be supported in targets and commands. Yes, of course. Cygwin runs unmodified UNIX scripts and makefiles, and they would screw up badly if ';' is used. I know about MSYS, but building an MSYS version of Make is already a special procedure. So I don't see why you should worry about that. I do not worry much, but i think a little about how this is done, because i suggest that my modification should not make this special procedure even more complicated. Don't you think so ? P.S. During testing Windows version of make i have found a little flaw in make test suite. In test_driver.pl there is a 'toplevel' function, which sets up environment for running make. And it has a list of env variables which are propagated. Can you add Windows-specific TEMP and TMP to it ? Without this Windows version fails back to '/' as temp directory, which fails miserably on post-XP because accessing C:\ requires elevated privilege. Sorry for not providing a .diff, but this is one-line fix, and approving it is IMHO too much overhead. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH] Use spawn() in GNU Make on Cygwin, updated
Hello! Once again, please make this a run-time option, off by default, activated by a command-line argument. Not a compile-time feature. Using spawn by default for Cygwin is a non-starter. Runtime ??? I am sorry, but what's the sense ? First, this will not work well, because many makefiles around call make recursively. And they will not necessarily pass over command line options. It is possible to modify make to do so, but this is horribly inconvenient. Well, it would be a little more convenient to have env variable, something like MAKE_USE_SPAWN, but anyway, what is the real purpose of keeping fork() ? Additionally your proposal would mean LOTS of changes. Instead of several #ifdef's you would have both versions of code plus some extra conditions. Why bloating up the code so much? spawn() is a full equivalent of fork() + exec(). Even more, in Cygwin exec() and spawn() internally use the same code. If exec() works, spawn() works too. BTW gcc on Cygwin uses spawn() for running its components, and it works quite fine. Also, you are now modifying the EMX parts of the code, and I at least have no way of knowing whether those changes will work well for EMX. So please guard them with a Cygwin-specific conditional. The changes are: 1. Rename 'status' to 'child_status'. Fixes error produced by new gcc, because of nested 'status' declarations. 2. Set environment to child's one before calling spawnvpe() and restore it back after exit from child_execute_job(). (1) is completely safe, it just fixes compile error. Nobody has hit this on EMX because nobody has ever tried to use gcc 4.8 on EMX because OS/2 is long dead. (2) should also be safe and at least harmless. Well, if you really want it, changing child's environment can be #ifdef'ed. However according to specification, spawnvpe() looks for the binary in current PATH, so on EMX the same thing should apply as on Cygwin. Well, it could work if EMX doesn't strictly follow POSIX semantics and implicitly considers current directory to be part of PATH (DOS/Windows-style behavior). OK, if you really-really don't care about what someone other than you think, can you accept last chunk of the patch for now (variable rename) ? This would decrease size of speedup patch which i will have to maintain. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH 2/2] Do not use DOS paths on Cygwin
Hello! In that case, we need to fix 'abspath', not disable this feature. Ok. If you don't mind, i can try new approach on this. Will it be OK ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH 2/2] Do not use DOS paths on Cygwin
Hello! In that case, we need to fix 'abspath', not disable this feature. I have came up with another implementation, currently testing it. My new implementation should allow DOS and UNIX paths to coexist. However there's still two differences: 1. abspath on Cygwin returns UNIX-style paths and on pure DOS/Windows - DOS-style. If DOS-style absolute path is already supplied, it will leave it as is. 2. PATH_SEPARATOR on Cygwin is ':' and on pure DOS/Windows is ';'. In order to distinguish between these two i have added one more test to configure. It attempts to compile and run a small program which checks whether result of getcwd() is DOS-style. In case of cross-compiling it just knows that for DOS and MinGW native path style is DOS. Of course config.cache value is taken into account. Technically it is possible to work around (2) using some clever heuristics, but i don't see how it would be possible to determine whether e.g. /foo/bar is an absolute path of DOS-style relative one. Will this solution be OK ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH] Use spawn() in GNU Make on Cygwin, updated
Hello! I have found and fixed the problem. It appeared to be PATH issue. fork()-based code temporary sets 'environ' variable to child's environment, which appears to contain current directory. EMX code didn't do that. The problem gets triggered only if you try to call something which is not in your system PATH. Cygwin's spawnvpe() screws up badly in such situation. The new patch is attached. I have tested building Cygwin source, works fine. Please test. If OK, i can retry pushing it upstream. -- Kind regards, Pavel Fedin make-3.82.90-1-use-spawn-on-cygwin.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
Re: [PATCH 2/2] Do not use DOS paths on Cygwin
Hello, Eli. Monday, August 5, 2013, 18:44:17 you wrote: I don't yet understand why you have a problem in the first place. It sounds like the single issue is with the 'abspath' function, is that correct? Yes, correct. 2 Christopher Faylor: ac_cv_dos_paths=no configure works just fine and does not require a command-line switch. [skip] I assume that the first time you wouldn't know about a command-line switch either. Sometimes you have to do research. Yes, but if this switch defaults to no on Cygwin, then i would not have to bother. Only those who want customized make with DOS paths would bother. At the other hand, according to your own logic, if we just change Cygwin's result for this test, then ac_cv_dos_paths=yes configure would also work for building customized version. But, okay, okay... I see, you are extremely conservative. Okay, i just wanted to make the little thing cleaner. AFAIK configure's purpose is to set up the source for building for particular system, and it should do all adjustments which are necessary to build the 100% working program. Customization (like enabling DOS paths for UNIXish environment) IMHO should not be done by default. But you are Masters, and if you completely disagree with me, let it be. I'm just a single person, and you are (at least) two. And you are maintainers and not me. And i am completely out of more arguments, so i retreat. I won't bother you with this particular issue any more, unless you become interested and explicitly ask me or suggest something. -- Kind regards, Pavel mailto:pavel_fe...@mail.ru ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
RE: [PATCH 2/2] Do not use DOS paths on Cygwin
Hello! Right. Because I knew I could just turn if off for the Cygwin release. There is no reason to nuke the feature for people who want to roll their own version of make with DOS paths turned on. Then maybe really add something like --enable-dos-paths which defaults to no on Cygwin and Yes on MinGW ? IMHO it's a little bit inconvenient that you have to use config.cache trick in order to build a fully working Make for Cygwin, and this is not documented anywhere. First time, when i didn't know about this, i've got a problem. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH1/2] Use spawn() on Cygwin
Hello! Please take this patch, Cygwin team told that they would like to integrate with upstream. I have already posted it some time ago but got no reply. The patch significantly improves performance of Make under Cygwin. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia make-3.82.90-1-use-spawn-on-cygwin.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
[PATCH 2/2] Do not use DOS paths on Cygwin
Currently make's configure suggests that it should use DOS-style paths on Cygwin. This is not true, and this assumption makes path-related mechanisms to work incorrectly. Currently Cygwin package supplies manual hint in config.cache in order to work around this. I think we should also ask MinGW team about __MSYS__ definition. MSys is close to Cygwin, so perhaps this should also be removed. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia make-3.82.90-1-no-dos-paths-on-cygwin.diff Description: Binary data ___ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make
