bugGNU Octave - Bugs: bug #53938, Windows Octave doesn't wait for...

 
 

bug #53938: Windows Octave doesn't wait for --eval statements to complete

Submitted by:  John Donoghue <lostbard>
Submitted on:  Fri 18 May 2018 02:05:45 PM UTC  
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  John Donoghue <lostbard>
Originator Name:  JohnD Open/Closed:  Closed
Release:  4.4.0 Operating System:  Microsoft Windows

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

( Jump to the original submission)

Mon 09 Jul 2018 10:24:21 PM UTC, comment #27:

I just built stable version 4.4.0x and verified the fix works.

Rik <rik5>
Project Administrator
Mon 09 Jul 2018 08:12:37 PM UTC, comment #26:

Okay, I merged stable to default here (http://hg.savannah.gnu.org/hgweb/octave/rev/a461dedb833f). I'm going to close this report.

Rik <rik5>
Project Administrator
Mon 09 Jul 2018 06:56:05 PM UTC, comment #25:

I will need do a new rebuild, but dev octave running:

system('C:/Octave/OCTAVE~4.0/bin/octave-5.0.0.bat --no-gui --norc --eval "pause(10); a=1"')

works for me.

Stable will also need to get merged back to default to get the changes

John Donoghue <lostbard>
Project MemberIn charge of this item.
Mon 09 Jul 2018 06:48:04 PM UTC, comment #24:

@JohnD: You originally reported the bug with this test code

Does it now work? If so, I think you can close this report.

Rik <rik5>
Project Administrator
Mon 09 Jul 2018 06:39:08 PM UTC, comment #23:

and to mxe
http://hg.octave.org/mxe-octave/rev/a800e350d88b

John Donoghue <lostbard>
Project MemberIn charge of this item.
Mon 09 Jul 2018 06:37:32 PM UTC, comment #22:

Pushed to stable
http://hg.savannah.gnu.org/hgweb/octave/rev/65a28dec405b

John Donoghue <lostbard>
Project MemberIn charge of this item.
Fri 06 Jul 2018 07:57:17 PM UTC, comment #21:

I think it does need to go on stable. The issue that motivated this report is that certain packages would fail to install correctly on stable. I suppose if there is a more minimal way to insure that we could use that solution, but it sounded like it would require review of every single Octave Forge package and swapping octave.exe with octave-cli.exe. In that case, I'd rather fix Octave core so that the packages can remain as written.

Rik <rik5>
Project Administrator
Fri 06 Jul 2018 07:27:43 PM UTC, comment #20:

So should the octave .bat changes goes into stable ?

John Donoghue <lostbard>
Project MemberIn charge of this item.
Fri 06 Jul 2018 03:04:05 PM UTC, comment #19:

updated patch to include the bash stub to the bat files

(file #44515)

John Donoghue <lostbard>
Project MemberIn charge of this item.
Fri 06 Jul 2018 01:42:56 PM UTC, comment #18:

ok is looking like I can somethinh at the topof the bat file like:

:; cmd.exe //c $0 "$@"; exit $
and it will be ignored bt cmd, but will then recall the script using com.exe

John Donoghue <lostbard>
Project MemberIn charge of this item.
Fri 06 Jul 2018 12:08:10 PM UTC, comment #17:

statistics is overriding octave.

I created the octave changes patch from the stable branch so at least it should merge cleanly.

However .....

I just tried the strings package which I see has $(shell $(OCTAVE)) call in its make file and it fails at that point because it is running in a bash shell and do doesn't know the DOC commands used in bat files

And to make more fun, from octave, running system("octave.bat") works fine as it is then using the windows shell.

Im not sure what other repercusions there would be if we just change the shell variable in the scripts

John Donoghue <lostbard>
Project MemberIn charge of this item.
Thu 05 Jul 2018 11:09:52 PM UTC, comment #16:

This works for me. The directory structure that Mike outlined is created correctly. I tried installing the io and statistics packages and both work. I used breakpoints to determine that octave_binary is correctly set to a .bat file.

I never saw the .bat file get used, however, when I built the two packages using 'pkg install -verbose XXX'. It may be that the requisite command was not echoed to screen, or it could mean that the individual package maintainers are overriding octave_binary.

I think we can commit this, but we will also need to backport the change to configure_make.m to the stable branch. That might upset people, I don't know.

Rik <rik5>
Project Administrator
Thu 05 Jul 2018 09:11:32 PM UTC, comment #15:

This will take me several hours, but I'll apply both patches and see what happens.

Rik <rik5>
Project Administrator
Thu 05 Jul 2018 06:20:21 PM UTC, comment #14:

and untested mod tfor pkg/private/configure_make.m to use .bat if windows.

(file #44512)

John Donoghue <lostbard>
Project MemberIn charge of this item.
Thu 05 Jul 2018 05:26:54 PM UTC, comment #13:

and the patch mentioned in comment #12 ....

(file #44511)

John Donoghue <lostbard>
Project MemberIn charge of this item.
Thu 05 Jul 2018 05:26:20 PM UTC, comment #12:

patch added that does find of octave-clie.exe similar to comment #11, and also removed/installs octave.bat and octave-version.bat

Tested in dev and appears to work ok.

WIth patch applied, pkg install no longer works as it looks for octave-XXXXXX.exe curring the initial config/make stage and cant find it

John Donoghue <lostbard>
Project MemberIn charge of this item.
Mon 02 Jul 2018 08:59:09 PM UTC, comment #11:

I think this probably requires John D. to make the necessary changes. However, I am attaching a start which puts octave.bat in the bin directory and makes the installer rely on finding octave-cli.exe rather than octave.exe to determine whether Octave exists.

(file #44495)

Rik <rik5>
Project Administrator
Fri 29 Jun 2018 09:39:04 PM UTC, comment #10:

I would prefer keeping octave-cli-4.4.0, because on other operating systems users can rely on both "${bindir}/octave-cli" and "${bindir}/octave-cli-${version}" existing. It's ok that octave-gui.exe is the oddball.

Mike Miller <mtmiller>
Project Administrator
Fri 29 Jun 2018 09:34:57 PM UTC, comment #9:

Mike's proposal in comment #8 seems sensible. For consistency, I would get rid of octave-cli-4.4.0.exe as well since there is no analog in the GUI such as octave-gui-4.4.0.exe. Thus, I'm left with

Should this change be done on stable before the 4.4.1 release, or is it too much change? If yes, should probably alert jwe to the work being done here.

Rik <rik5>
Project Administrator
Fri 29 Jun 2018 09:06:14 PM UTC, comment #8:

To summarize and hopefully clarify, we now have

I think we should have this instead

And optionally keep the copy of octave.bat at the top-level if that is considered useful.

Mike Miller <mtmiller>
Project Administrator
Fri 29 Jun 2018 08:56:50 PM UTC, comment #7:

We would also have to copy octave.bat into the bin directory. It currently only sits at the Octave root directory (OCTAVE_HOME). We would also have to copy octave.bat to octave-4.4.0.bat so that the versioned program name can be used.

I still advocate for deleting octave.exe completely. It should not be used at all. Both octave.bat and octave.vbs call octave-cli.exe or octave-gui.exe directly, octave.exe is just wasting space and adding confusion for users who poke around in the directory.

Mike Miller <mtmiller>
Project Administrator
Fri 29 Jun 2018 08:49:07 PM UTC, comment #6:

Any thoughts on this bug? It seems like it should be resolvable without TOO much trouble.

If I understand John D. correctly, Windows prefers the .exe extension to the .bat extension when the same base file name exists.

Two easy possibilities suggest themselves:

1) On Windows only (ispc() or some such test), change definition of octave_binary so that ext=".bat" in the following code.

2) Alternatively, rename true executable to something like "octave.exe.exe". In this case, calling just "octave" would launch "octave.bat" since the basename of the executable is now "octave.exe". This would require updating the .bat file to launch the correct executable, but hopefully the installed desktop shortcuts were already launching the .bat file so there would be no changes there.

Rik <rik5>
Project Administrator
Thu 24 May 2018 11:48:51 AM UTC, comment #5:

Added change to start octave without 'start' from octave.bat when non gui mode.
http://hg.octave.org/mxe-octave/rev/d1d36bdbc1ae

John Donoghue <lostbard>
Project MemberIn charge of this item.
Mon 21 May 2018 08:13:06 PM UTC, comment #4:

I just checked on my win 10 machine - it wont try .BAT before .EXE if not extenstion is provided

John Donoghue <lostbard>
Project MemberIn charge of this item.
Mon 21 May 2018 06:33:18 PM UTC, comment #3:

I think we should make it so that calls to ${OCTAVE_HOME}/bin/octave and ${OCTAVE_HOME}/bin/octave-${VERSION} call the octave.bat script instead of the octave.exe program. The purpose of octave.bat is to replace octave.exe, so I think it should replace it in all instances.

Inside of Octave, the user should be able to rely on ${OCTAVE_HOME}/bin/octave-${VERSION} being a well-known entry point that can be called in a useful way regardless of the operating system.

If octave.bat is copied into bin/octave.bat and bin/octave-4.4.0.bat, does it automatically take precedence over bin/octave.exe and bin/octave-4.4.0.exe? Then the only change necessary in configure_make would be to drop the exe extension.

Mike Miller <mtmiller>
Project Administrator
Fri 18 May 2018 07:06:08 PM UTC, comment #2:

yes to the last part of the comment with the OCTAVE=, which is really the issue I was trying to communicate on the bug report :).

Doing a pkg install calls configure_make.m, which sets the variables that configure and make will use:

cenv = {"MKOCTFILE"; mkoctfile_program;
"OCTAVE_CONFIG"; octave_config_program;
"OCTAVE"; octave_binary};

where octave_binary = fullfile (octave_bindir, sprintf ("octave-%s%s", ver, ext));

and then for configure:

cmd = ["cd '" src "'; " scenv "./configure " flags];

and for make:

sprintf ("%s make --jobs %i --directory '%s'", scenv, jobs, src)

So anyone writing a makefile that takes notice of the OCTAVE value, will be using octave-4.0.0, rather than octave-cli.

For the make octave.bat call 'octave-cli.exe' o=comment, I'm fine with that - cmdshell.bat should probally also be updated to include the base install path in its path so that octave.bat will be found from the command line.

John Donoghue <lostbard>
Project MemberIn charge of this item.
Fri 18 May 2018 04:41:30 PM UTC, comment #1:

I think everyone agrees that 'octave.exe --no-gui' should not be used. In fact, I thought it was also generally agreed on that octave.exe should never be used, that's why we have octave.bat and octave.vbs. And that's why the '--no-gui' as an option runs octave-cli instead of octave-gui.

I would say on GNU and Unix systems there are three modes, 'octave-cli', 'octave --no-gui' (default), and 'octave --gui', but on Windows there are only two modes that work. If we can make that easier somehow by adding options to the bat file or deleting the octave.exe that users shouldn't be using, we should do so.

IMHO calling 'octave --no-gui --no-window-system' should end up executing 'octave-cli' on all platforms.

Can we make octave.bat call 'octave-cli.exe' insetad of 'start octave-cli.exe' as a first step?

What do you mean by calling octave-cli "doesnt get passed through the pkg build system as a variable"? Meaning that the configure script still gets called with OCTAVE=$fullpath/octave-4.4.0?

Mike Miller <mtmiller>
Project Administrator
Fri 18 May 2018 02:05:45 PM UTC, original submission:

When running a package install that relies on doing something with octave via the $(OCTAVE) variable, the octave process doesnt wait for the actual operation to finish before returing control.

Outside of the package build, running something like:

system('C:/Octave/OCTAVE~2.0/bin/octave-4.4.0.exe --no-gui --norc --eval "sleep(10); a=1"')

the system call returns long before the eval statements were complete.

Perhaps a 'feature' of windows, however as mentioned above, in package install scripts, if relying on doing something using the OCTAVE variable in the makefile, it doesnt work.

Alternative is to use octave-cli, which does wait however that doesnt get passed through the pkg build system as a variable.

John Donoghue <lostbard>
Project MemberIn charge of this item.

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #44515:  use_bat_not_exe2.patch added by lostbard (5KiB - application/octet-stream)
file #44512:  octave-pkg-bat.patch added by lostbard (1KiB - application/octet-stream)
file #44511:  use_bat_not_exe.patch added by lostbard (5KiB - application/octet-stream)
file #44495:  winbat.patch added by rik5 (2KiB - 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 lostbard
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by lostbard (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-09 rik5 StatusPatch Reviewed => Fixed
        Open/ClosedOpen => Closed
    2018-07-06 lostbard Carbon-Copy- => Added jwe
    2018-07-06 lostbard Attached File- => Added use_bat_not_exe2.patch, #44515
    2018-07-05 rik5 StatusPatch Submitted => Patch Reviewed
    2018-07-05 rik5 SummaryWindows octave doesnt wait for --eval statements to complete => Windows Octave doesn't wait for --eval statements to complete
    2018-07-05 lostbard Attached File- => Added octave-pkg-bat.patch, #44512
    2018-07-05 lostbard StatusIn Progress => Patch Submitted
    2018-07-05 lostbard Attached File- => Added use_bat_not_exe.patch, #44511
    2018-07-05 lostbard StatusPatch Submitted => In Progress
        Assigned toNone => lostbard
    2018-07-02 rik5 Attached File- => Added winbat.patch, #44495
        StatusNone => Patch Submitted
    2018-06-29 rik5 CategoryNone => Configuration and Build System
    2018-06-29 rik5 Summarywindows octave doesnt wait for to --eval statements to complete => Windows octave doesnt wait for --eval statements to complete

    Back to the top


    Powered by Savane 3.3