bugGNU Octave - Bugs: bug #57735, pkg: package names should be...

 
 

bug #57735: pkg: package names should be handled case insensitively

Submitter:  Mike Miller <mtmiller>
Submitted:  Mon 03 Feb 2020 11:21:21 PM UTC
   
 
Category:  Octave Function Severity:  1 - Wish
Priority:  1 - Later Item Group:  Feature Request
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 01 Jul 2020 03:01:29 PM UTC, comment #9: 

Understood.  It makes sense, but I wish this were easier to implement.

Rik <rik5>
Group administrator
Tue 30 Jun 2020 09:43:49 PM UTC, comment #8: 

As the reporter my intent is exactly that case shouldn't matter at all. I should be able to create a package with the name declared as "ExampleData", and a user should be able to install it, uninstall it, and load it with the name "ExampleData" or "exampledata" or "eXaMpLeDaTa".

Mike Miller <mtmiller>
Group Member
Tue 30 Jun 2020 02:46:59 PM UTC, comment #7: 

Do we have a sense about whether package names should be lower case?  I took another look at this and just treating things case insensitively, such as swapping all strcmp for strcmpi, is not likely to be enough.  If someone wants the "nan" package from Octave-Forge, but types "NaN", strcmpi will compare true but there is no NaN package to download.  Sure, this can be worked around by getting the list, doing the comparison case insensitively, and then always choosing the name from the download list.  But that is just one example.

It would be simpler if we had a convention, such as all package names are lower case, but it might be too late too implement that if there are a lot in the wild with mixed case already.

Rik <rik5>
Group administrator
Wed 24 Jun 2020 02:43:13 PM UTC, comment #6: 

Okay, I guess this bug will require a bit more thought.  For the time being I backed out the previous changeset.

Rik <rik5>
Group administrator
Wed 24 Jun 2020 08:12:36 AM UTC, comment #5: 

Two tests are failing on the buildbots since this change:

>>>>> processing /scratch/buildbot/workers/jwe-debian-x86_64-1/clang-4_0-debian/src/test/pkg/pkg.tst
***** testif HAVE_Z
 ## Do all tests in a temporary directory
 [old_prefix, old_archprefix] = pkg ("prefix");
 restorecfg = onCleanup (@() pkg ("prefix", old_prefix, old_archprefix));
 old_local_list = pkg ("local_list");
 restorecache = onCleanup (@() pkg ("local_list", old_local_list));
 old_global_list = pkg ("global_list");
 restoreglobalcache = onCleanup (@() pkg ("global_list", old_global_list));
 prefix = tempname ();
 [status] = mkdir (prefix);
 if (! status)
   error ("pkg.tst: Could not create temporary directory for pkg testing");
   return;  # abort further testing
 endif
 pkg ("prefix", prefix, prefix);
 pkg ("local_list", fullfile (prefix, "octave_packages"));
 pkg ("global_list", fullfile (prefix, "octave_packages"));
 rmtmpdir = @onCleanup (@() test_cleanup (prefix));
 ## Create tar.gz file packages of testing directories in prefix directory
 mfile_pkg_name = {"mfile_basic_test", "mfile_minimal_test"};
 mfile_pkg_tar = fullfile (prefix, strcat (mfile_pkg_name, ".tar"));
 mfile_pkg_tgz = strcat (mfile_pkg_tar, ".gz");
 for i = 1:numel (mfile_pkg_name)
   tar (mfile_pkg_tar{i}, mfile_pkg_name{i});
   gzip (mfile_pkg_tar{i});
 endfor
!!!!! test failed
pkg: cannot create file /tmp/oct-vhqn0c/octave_packages
***** testif HAVE_Z
 silent_pkg_install ("-local", mfile_pkg_tgz{1});
 [desc, flag] = pkg ("describe", mfile_pkg_name{1});
 ## FIXME: this only tests that the describe command runs,
 ##        not that the output is in anyway correct.
 system (["chmod -Rf u+w '" prefix "'"]);     ## FIXME: Work around bug #53578
 pkg ("uninstall", mfile_pkg_name{1});
!!!!! test failed
matrix cannot be indexed with {


Might be because input should be case sensitive for "prefix" (and maybe others).

Markus Mützel <mmuetzel>
Group administrator
Tue 23 Jun 2020 06:55:56 PM UTC, comment #4: 

I checked in my change from comment #2 here: https://hg.savannah.gnu.org/hgweb/octave/rev/9e9ddc90e88a.

Marking as Ready for Test.

Rik <rik5>
Group administrator
Wed 10 Jun 2020 11:00:30 AM UTC, comment #3: 

Just FTR, this bug report was motivated by remarks in comment #29 and later in bug #57522

Philip Nienhuis <philipnienhuis>
Group Member
Tue 09 Jun 2020 09:44:10 PM UTC, comment #2: 

Is a fix as simple as using tolower() early on when names are collected?


diff -r f5c9bb5955e7 scripts/pkg/pkg.m
--- a/scripts/pkg/pkg.m        Tue Jun 09 14:11:13 2020 -0700
+++ b/scripts/pkg/pkg.m        Tue Jun 09 14:41:44 2020 -0700
@@ -456,7 +456,7 @@ function [local_packages, global_package
         endif
         action = varargin{i};
       otherwise
-        files{end+1} = varargin{i};
+        files{end+1} = tolower (varargin{i});
     endswitch
   endfor





(file #49235)

Rik <rik5>
Group administrator
Mon 03 Feb 2020 11:22:37 PM UTC, comment #1: 

This does not need to be addressed for Octave 6, and we can discuss and collect comments either here or on the maintainers mailing list.

Mike Miller <mtmiller>
Group Member
Mon 03 Feb 2020 11:21:21 PM UTC, original submission:  

The package name tends to be lowercase, but the code in pkg.m and its subfunctions handles the package name inconsistently. Sometimes it needs to match exactly, including case, and sometimes the case does not matter.

In my opinion, case should be ignored in all cases, all comparisons with package names should be case insensitive. This would be consistent with the 'pip' package manager for Python.

Example:


>> pkg install -forge Signal
For information about changes from previous versions of the signal package, run 'news signal'.
>> pkg install -forge SIGNAL
For information about changes from previous versions of the signal package, run 'news signal'.
>> pkg install -forge sIgNaL
For information about changes from previous versions of the signal package, run 'news signal'.
>> pkg load sIgNaL
error: package sIgNaL is not installed

>> pkg uninstall sIgNaL
warning: some of the packages you want to uninstall are not installed

>> pkg load SIGNAL
error: package SIGNAL is not installed

>> pkg unload SIGNAL
error: pkg: package(s): SIGNAL not installed

>> pkg uninstall SIGNAL
warning: some of the packages you want to uninstall are not installed

>> pkg describe Signal
error: some packages are not installed:  Signal



I think all of these should work equivalently regardless of the case the user types or the case that the package maintainer chose to put in the DESCRIPTION file.

Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #49235:  bug57735.diff added by rik5 (384B - 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 mmuetzel (Posted a comment)
  • -email is unavailable- added by rik5 (Updated 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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-06-30 mtmiller StatusNeed Info Confirmed
    2020-06-30 rik5 StatusReady For Test Need Info
    2020-06-23 rik5 StatusConfirmed Ready For Test
    2020-06-09 rik5 Attached File- Added bug57735.diff, #49235
        StatusNone Confirmed
    2020-02-03 mtmiller Priority5 - Normal 1 - Later
    2020-02-03 mtmiller Carbon-CopyRemoved 80942 -

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code