bugmake - Bugs: bug #53879, Support for .ONESHELL target on...

 
 

bug #53879: Support for .ONESHELL target on Windows and DOS platforms

Submitter:  V. Aleksandrov <valeksan>
Submitted:  Fri 11 May 2018 03:59:55 PM UTC
   
 
Severity:  3 - Normal Item Group:  Enhancement
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Component Version:  4.2.1 Operating System:  MS Windows
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 15 Aug 2018 09:31:29 AM UTC, comment #5: 

Indeed, the DJGPP library provides tempnam() and tmpnam() for generation of a random file name. An implementation of function create_batch_file() that uses
aforementioned functions would be simpler.

The advantage of the pseudo-random file name generator is that the source code is independent on the presence of such library functions. Other compilers rely on
libraries that lack optimized functions for file name generation.

Here is a more detailed explanation why the code is removed from file variable.c.

Let consider a makefile taken from the test suite:


.RECIPEPREFIX = >
.ONESHELL:
SHELL = perl
.SHELLFLAGS = -e
all:
>           @$$a=5
>            +7;
>        @y=qw(a b c);
>print "a = $$a, y = (@y)\n";


If the shell is specified without extension then function find_and_set_default_shell() called from variable.c would not set the shell variable to perl because
it can't find the executable in the path environment. Then sh.exe is used which can't interpret the script.

If the shell is specified with extension as `SHELL = perl.exe' then the perl executable is found in the path.

My understanding is to not have separate code for Windows from the code for Unix. So removing the code from file variable.c logically solves the issue as the shell
variable is initialized to `perl'. This is enough information for the process creation function at a later stage to find the perl executable.

Now, we are able to suggest that appending an extension when such is not given to the required shell in function find_and_set_default_shell() while preserving
the Windows specific code in variable.c would resolve the issue as well.

V. Aleksandrov <valeksan>
Sat 11 Aug 2018 07:56:46 AM UTC, comment #4: 

The MSDOS (a.k.a. "DJGPP") development environment certainly does have a capability of generating a temporary file name: use the 'tmpnam' or 'tempnam' library functions.

Eli Zaretskii <eliz>
Group Member
Sun 05 Aug 2018 12:49:13 PM UTC, comment #3: 

Thanks for the patch it would be great to have this work on Windows.  I agree with Eli that as it is the patch is large enough to require paperwork.  If you're interested in that please send me email <psmith@gnu.org>.  These days it can all be done electronically in most countries.

It seems one major source of work is a temp filename generator in create_batch_file(); does MSDOS not have any pre-existing function?  Maybe this could be simplified; I doubt we need a truly randomized file name generator on DOS as it's not really a multiprocessing system so doesn't have the same security issues.

In any event I'd prefer to see that system-specific code moved to a different file but we can handle that ourselves if you prefer.

The thing I didn't quite understand was the variable.c change which removes a bunch of code... that code is not moved anywhere that I can see so I'm not sure whether it's just not needed at all, or what?  Maybe Eli can look at that to better understand it.

There are a few nits such as adding a declaration of _is_dos_shell() to makeint.h when that's actually just a preprocessor macro in job.c, etc. but these are easy to fix.

Paul D. Smith <psmith>
Group administrator
Thu 07 Jun 2018 04:12:33 PM UTC, comment #2: 

On Windows platform, official GNU make handles properly .ONESHELL special target with Bourne or native (cmd.exe) shells.
However, if another shell (e.g. perl) is used, recipes can't be executed correctly.

The reason is that function do_variable_definition() assigns a shell variable to its default value (sh.exe). This is avoided by
removing conditional code in file variable.c.
In addition, `else' clause in file job.c line 3009 that handles non-Posix shell must not be executed. More precisely, the `else' block
is executed only when native shells are used by introducing an `else-if' clause.

On MS-DOS platform, the patch allows for .ONESHELL target only with the Bourne or native shells.

V. Aleksandrov <valeksan>
Sat 19 May 2018 08:49:08 AM UTC, comment #1: 

Thank you for your contribution, I will review it when I have time.

Can you post a short description of the main idea(s) of the patch?

Also, I think the patch is long enough to require legal paperwork, but that's for Paul Smith to decide.

Eli Zaretskii <eliz>
Group Member
Fri 11 May 2018 03:59:55 PM UTC, original submission:  

Please consider the proposed patch that implements .ONESHELL target on Windows and MS-DOS platforms.

V. Aleksandrov <valeksan>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #44140:  oneshell.patch added by valeksan (9KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by eliz (Posted a comment)
  • -email is unavailable- added by valeksan (Submitted the item)
  •  

    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 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-12-26 psmith Item GroupBug Enhancement
    2018-05-11 valeksan Attached File- Added oneshell.patch, #44140

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code