bugGNU Octave - Bugs: bug #65808, "octave.bat" does not...

 
 

bug #65808: "octave.bat" does not work in case the PATH variable contains directories with spaces

Submitter:  None
Submitted:  Wed 29 May 2024 07:14:33 PM UTC
   
 
Category:  Other Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Need Info Assigned to:  None
Originator Name:  Michael Bachmann Originator Email:  -email is unavailable-
Open/Closed:  * Open Release:  * 9.1.0
Operating System:  * Microsoft Windows Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 31 May 2024 08:45:13 PM UTC, comment #6: 

Hello Markus,

please find attached an updated version of installer-files.patch which includes the line end conversion by unix2dos and also includes the patching of the file post-install.bat.

Hope this makes it easier to apply.

Best regards
Michael

(file #56124, file #56125)

Anonymous
Fri 31 May 2024 05:24:32 PM UTC, comment #5: 

Hello Markus,

yes, the line ends of these files have been changed using unix2dos.

Please use the files in folder "modified/installer-files" for the update instead of installer_files.patch.

See details below.

Best regards
Michael


I downloaded the sources from the Mercurial repository website via the "raw" links.
Then I applied unix2dos to all four files since the patches:
cmdshell-no-short-path.patch
octave-bat-no-short-path.patch

could not be applied directly using "patch" command under cygwin:

$ patch -p1 < octave-bat-no-short-path.patch
patching file octave.bat
Assertion failed: hunk, file .\src\patch\2.5.9\patch-2.5.9-src\patch.c, line 354

$ patch -p1 < cmdshell-no-short-path.patch
patching file cmdshell.bat
Assertion failed: hunk, file .\src\patch\2.5.9\patch-2.5.9-src\patch.c, line 354


So I applied unix2dos to all four files:

$ unix2dos *
unix2dos: converting file cmdshell-no-short-path.patch to DOS format...
unix2dos: converting file cmdshell.bat to DOS format...
unix2dos: converting file octave-bat-no-short-path.patch to DOS format...
unix2dos: converting file octave.bat to DOS format...

=> The files which have been processed by unix2dos have been used then for applying the fix with the double quotes.

Then the patches from the repository could be applied:

$ patch -p1 < cmdshell-no-short-path.patch
patching file cmdshell.bat
$ patch -p1 < octave-bat-no-short-path.patch
patching file octave.bat
Hunk #2 succeeded at 66 (offset 25 lines).

Anonymous
Fri 31 May 2024 09:56:57 AM UTC, comment #4: 

Where did you get the base sources from? And how did you create the patch?
I tried to apply it. But it didn't work for me:

$ patch -Nbp1 -i installer_files.patch
patching file installer-files/cmdshell-no-short-path.patch
Hunk #1 FAILED at 11 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file installer-files/cmdshell-no-short-path.patch.rej
patching file installer-files/cmdshell.bat
Hunk #1 FAILED at 1 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file installer-files/cmdshell.bat.rej
patching file installer-files/octave-bat-no-short-path.patch
Hunk #1 FAILED at 11 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file installer-files/octave-bat-no-short-path.patch.rej
patching file installer-files/octave.bat


In case you are checking out the Mercurial repository, make sure to check-out line endings as they are used in the repository.

Markus Mützel <mmuetzel>
Group administrator
Fri 31 May 2024 08:36:51 AM UTC, comment #3: 

Thank you for the patch for MXE Octave. I haven't tested it yet. But I'll try to do so soon.

To push that to repository, we'd need a name and email address of the author. Can I use the name and email that you supplied for this bug report for your contribution?
If you prefer to stay anonymous in the repository history, I could also push the change with my name.

When it comes to dropping the patches and applying them to the batch files also for releases: I'm not sure whether we are there yet. While spaces in paths should be handled correctly in many (most of?) the places in Octave by now, there are still a couple of places where I don't know if that would work (e.g., when calling the compiler/linker from mkoctfile or when using configure scripts or Makefiles when installing packages, e.g., with `pkg install -forge package_name`, and probably more places where that doesn't work yet). I'm not even sure if all of these issues can be solved in Octave itself. Some might need changes to the build rules in Makefiles or configure scripts that are distributed by the maintainers of the respective Octave packages...
So, for the time being at least, it is probably safest to continue carrying these patches and only applying them when building `default-octave` (but not for stable or release).

In general, uploading patches here is enough. You don't need to also upload the changed files.

Markus Mützel <mmuetzel>
Group administrator
Thu 30 May 2024 10:39:47 PM UTC, comment #2: 

Hello Markus,

please find attached an updated version of the patch for all four files of the installer-files folder:

cmdshell-no-short-path.patch
cmdshell.bat
octave-bat-no-short-path.patch
octave.bat

Since the patch files:

cmdshell-no-short-path.patch
octave-bat-no-short-path.patch

Remove the 8.3 conversion and the following comment in octave.bat

Rem Convert to 8.3 format so we don't have to worry about spaces.

suggests that the conversion has been applied to work around spaces, these patches might not be necessary anymore.

So it might be possible to use the files cmdshell.bat and octave.bat in the subfolder:

patches_applied_which_remove_the_8.3_conversion

directly and skip the patch files:

cmdshell-no-short-path.patch
octave-bat-no-short-path.patch

Best regards
Michael

(file #56118, file #56119)

Anonymous
Thu 30 May 2024 08:39:57 AM UTC, comment #1: 

Thank you for the report. Your proposed changes look reasonable to me.

The original .bat file is in the repository for MXE Octave:
https://hg.octave.org/mxe-octave

In particular, it is this file:
https://hg.octave.org/mxe-octave/file/09b1697eff3a/installer-files/octave.bat

The following patch probably also needs changes if the .bat file is changed:
https://hg.octave.org/mxe-octave/file/09b1697eff3a/installer-files/octave-bat-no-short-path.patch

Would you like to prepare a patch for that repository? (If you prefer not to, I could try do look into that part.)

Additionally, would it make sense to make similar changes to the `cmdshell.bat` script?
https://hg.octave.org/mxe-octave/file/09b1697eff3a/installer-files/cmdshell.bat

In case it would, the following patch would probably also need to be adapted:
https://hg.octave.org/mxe-octave/file/09b1697eff3a/installer-files/cmdshell-no-short-path.patch

Please, let us know if you'd like to propose a patch for that repository.

Markus Mützel <mmuetzel>
Group administrator
Wed 29 May 2024 07:14:33 PM UTC, original submission:  

In case e.g. the PATH variable contains directories with spaces e.g.:

C:\Program Files (x86)\NVIDIA Corporation\PhysX\Common

the following error occurs:

>set "PATH=C:\Program Files\GNU Octave\Octave-9.1.0\mingw64\bin;%PATH%"
>octave.bat --version

\NVIDIA was unexpected this time.


The solution for the issue is to put the argument of every call of "set" in double quotes like e.g.:

set "PATH=%OCT_HOME%\bin;%PATH%"

This allows to handle variable values which contain spaces correctly.

Please find attached a patched version of octave.bat in which the double quotes have been added.

Using the patched version of octave.bat:

>octave.bat --version

GNU Octave, version 9.1.0
Copyright (C) 1993-2024 The Octave Project Developers.
This is free software; see the source code for copying conditions.
There is ABSOLUTELY NO WARRANTY; not even for MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE.

Octave was configured for "x86_64-w64-mingw32".

Home page:            https://octave.org
Support resources:    https://octave.org/support
Improve Octave:       https://octave.org/get-involved

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #56124:  installer-files.patch added by None (16KiB - application/octet-stream - installer-files.patch: (raw=>fmodified) includes line end conversion (unix2dos) and the patch for post-install.bat; octave-9.1.0.zip: The raw and modified files;)
file #56125:  octave-9.1.0.zip added by None (21KiB - application/x-zip-compressed - installer-files.patch: (raw=>fmodified) includes line end conversion (unix2dos) and the patch for post-install.bat; octave-9.1.0.zip: The raw and modified files;)
file #56118:  octave-9.1.0.zip added by None (16KiB - application/x-zip-compressed - octave-9.1.0.zip: original and modified files; installer_files.patch: updated patch file for the files in folder installer-files)
file #56119:  installer_files.patch added by None (7KiB - application/octet-stream - octave-9.1.0.zip: original and modified files; installer_files.patch: updated patch file for the files in folder installer-files)
file #56111:  octave-9.1.0.zip added by None (3KiB - application/x-zip-compressed - octave-9.1.0.zip contains the patched *.bat files and octave-9.1.0_bat.patch contains the patch.)
file #56112:  octave-9.1.0_bat.patch added by None (5KiB - application/octet-stream - octave-9.1.0.zip contains the patched *.bat files and octave-9.1.0_bat.patch contains the patch.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by None (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 group members can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-05-31 None Attached File- Added installer-files.patch, #56124
        Attached File- Added octave-9.1.0.zip, #56125
    2024-05-30 None Attached File- Added octave-9.1.0.zip, #56118
        Attached File- Added installer_files.patch, #56119
    2024-05-30 mmuetzel CategoryOctave Package Other
        StatusNone Need Info
    2024-05-29 None Attached File- Added octave-9.1.0.zip, #56111
        Attached File- Added octave-9.1.0_bat.patch, #56112

    Back to the top

    Powered by Savane 3.13-c54d.
    Corresponding source code