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

 
 

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

Submitted by:  None
Submitted on:  Thu 16 Jan 2014 05:58:28 AM UTC  
 
Severity: 3 - NormalItem Group: None
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Component Version: SCMOperating System: MS Windows
Fixed Release: NoneTriage Status: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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 -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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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 File(s):
   
   
Comment:
   

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 31 Jan 2014 09:14:31 AM UTCglandiumAttached File-=>Added diff, #30424
    Thu 16 Jan 2014 05:58:28 AM UTCNoneAttached File-=>Added diff, #30286

    Back to the top


    Powered by Savane 3.1-cleanup