bugmake - Bugs: bug #41246, Allow to switch shell batch mode...

 
 

bug #41246: Allow to switch shell batch mode at runtime instead of build time

Submitter:  None
Submitted:  Thu 16 Jan 2014 05:58:28 AM UTC
   
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Component Version:  4.1 Operating System:  MS Windows
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 31 Jan 2014 12:24:58 PM UTC, comment #11: 

Hi Mike,

Sorry to hijack this thread a little.

I work on the MSYS2 project and I'd be keen to fix "sh.exe -c" to behave correctly. If you are interested in this, could you please email me at -email is unavailable- with a testcase?

FWIW I agree that being able to punt the shell commands into a single script executed via sh.exe is a good feature to have. I'd also like to be able to keep them around afterwards for inspection or to re-run them, like GCC's --save-temps argument.

Ray Donnelly <raydonnelly>
Fri 31 Jan 2014 09:14:31 AM UTC, comment #10: 

This is a different approach to the problem, as suggested by Paul: this triggers batch mode shell when there are double quotes in the recipe. Note this doesn't quite work around all the problems that the previous patch did solve when adding .BATCH_MODE_SHELL to the problematic makefile. There are still issues with backslashes, which I suspect are due to bad escaping when running with sh -c. Or discrepancy between sh -c and batch mode which leads to that particular Makefile miraculously working in batch mode. I'll need to do more testing.

(file #30424)

Mike Hommey <glandium>
Sat 18 Jan 2014 11:27:40 PM UTC, comment #9: 

Also we do sometimes see people complaining about this, where they can't invoke commands because the command line is too long.  There are various tricks out there for "chunking" make variable values containing lots and lots of words into smaller groupings.  It has to be done in make itself, obviously.

However, I will admit it's been a while since I've noticed a complaint about this.  Either people have gotten used to it or it's become less of a problem.

Paul D. Smith <psmith>
Group administrator
Sat 18 Jan 2014 11:24:33 PM UTC, comment #8: 

I understood that difference; that's why I said "The downside of this is that ONESHELL actually behaves differently".  However often this doesn't matter so much, particularly on Windows where the rules about what operations impact the parent's environment are not so strict as on POSIX systems (IIRC).  I was wondering if it was "close enough" to be acceptable.

In any event, I'd really prefer this be made to "just work" without special compile options or pseudo targets needed.  Is there a reason why that's not feasible?

Paul D. Smith <psmith>
Group administrator
Sat 18 Jan 2014 10:54:31 PM UTC, comment #7: 

I just realized I said something very wrong: BATCH_MODE_ONLY_SHELL is not all all like ONESHELL. It still runs a shell per recipe line. Sorry for the confusion.

Mike Hommey <glandium>
Sat 18 Jan 2014 10:51:43 PM UTC, comment #6: 

The command line size limit is in the order of several megabytes nowadays on Linux. It used to be much less, though. Arguably, if this was a wanted feature, you would likely have heard people complain when the command line size limit was lower.

Mike Hommey <glandium>
Sat 18 Jan 2014 09:17:25 PM UTC, comment #5: 

I'm not sure I like the idea of this being a special target in this way.  It's also possible that something like this COULD be used on UNIX systems; there are limits to the size of command lines that can be invoked.  On newer systems like Linux that limit is pretty large (I think it's 16K or so on Linux) but it exists, and on other systems it's much less.

There's long been requests for "@-file" support embedded in make although exactly what form that would take hasn't been defined clearly IIRC.

Let me ask this: would it be possible to simply use the existing ONESHELL capability and just say that on Windows it writes the script to a file and invokes the shell on it?

The downside of this is that ONESHELL actually behaves differently (although often this doesn't matter).

Also, if the "normal" behavior doesn't handle something as basic as quoting properly, why is BATCH_MODE_ONLY_SHELL not the default?  Is there a downside to it?

Or alternatively, if we detect that the script has content that might be problematic (quotes) we could automatically use the batch mode.

I guess I'd just prefer things to work right automatically, without recompiling make or adding new pseudo targets.  Is there a reason this is tricky or difficult?

Paul D. Smith <psmith>
Group administrator
Fri 17 Jan 2014 10:43:18 PM UTC, comment #4: 


> why not .BATCH_MODE_ONLY_SHELL, btw?


Apart from making it the same as the macro, the "only" feels too much for a target name. I don't care that much for the name, though, your pick.

> which should at least be documented in the manual.


Would you mind doing this for me?

> can someone describe for me what BATCH_MODE_ONLY_SHELL does? Does is overlap with ONESHELL?


In essence, this is like ONESHELL, except it writes the script to a file to then run "sh thatfile" instead of doing sh -c "line1\nline2\nline3". This is useful because msys bash on windows doesn't handle double quotes properly when run with sh -c. I don't think non-Windows systems would need this as a feature.

Mike Hommey <glandium>
Fri 17 Jan 2014 02:57:41 PM UTC, comment #3: 

Not being familiar with the various different options for shell behavior on Windows, can someone describe for me what BATCH_MODE_ONLY_SHELL does?  Does is overlap with ONESHELL?  If not is it a feature that might be useful to people on non-Windows systems?  I could read the code but a short primer would be nice :-)

Paul D. Smith <psmith>
Group administrator
Fri 17 Jan 2014 02:04:08 PM UTC, comment #2: 

The changes seem simple enough and non-controversial.  However, this introduces a new special target, .BATCH_MODE_SHELL (why not .BATCH_MODE_ONLY_SHELL, btw?), which should at least be documented in the manual.

Paul, do you agree with adding a new target?

Eli Zaretskii <eliz>
Group Member
Thu 16 Jan 2014 05:59:34 AM UTC, comment #1: 

I happen to have not been logged in when I submitted, but this bug and patch are mine :)

Mike Hommey <glandium>
Thu 16 Jan 2014 05:58:28 AM UTC, original submission:  

I've been successfully using a pristine build of make 4.0 on windows to build Firefox for a few weeks now, after avoiding the sh.exe issues by using single quotes instead of double quotes, but we unfortunately have some other nasty makefiles that are very hard to fixup this way without spending a huge amount of time rewriting it, and i would very much like to be able to just make that one makefile use a pristine build of make 4.0 as if it were built with BATCH_MODE_ONLY_SHELL defined.

The attached patch allows us to special-case that specific Makefile.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #30424:  diff added by glandium (2KiB - application/octet-stream - Different attempt)
file #30286:  diff added by None (1KiB - application/octet-stream - Proposed patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by raydonnelly (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by eliz (Posted a comment)
  • -email is unavailable- added by glandium (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-12-27 psmith Item GroupNone Enhancement
    2014-01-31 glandium Attached File- Added diff, #30424
    2014-01-16 None Attached File- Added diff, #30286

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code