bugGNU Octave - Bugs: bug #60574, pkg update ignores -global /...

 
 

bug #60574: pkg update ignores -global / -local options

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Mon 10 May 2021 05:59:58 PM UTC
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Nicholas Jankowski Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Thu 02 Dec 2021 06:49:52 PM UTC, comment #8: 
Nicholas Jankowski <nrjank>
Project Member
Thu 02 Dec 2021 04:20:24 PM UTC, comment #7: 

i don't think there are any other changes to make, only changes since May has been updating the docstring and refreshing to the current codebase.

I realize that this is an expanded feature, but the main intent back in May was to allow pkg update to be usable again for updating bundled packages, in particular in the windows zip versions of octave. Those are 'installable' in userspace but after v6 'pkg update' stopped being possible in userspace.  Would be nice to get that back without waiting for v8.

will walk through the code again and do a few local tests, and if no changes or concerns will push later today.

Nicholas Jankowski <nrjank>
Project Member
Thu 02 Dec 2021 09:17:28 AM UTC, comment #6: 

@Nik: If you think there is still enough time for stabilizing these changes before the release of Octave 7, go ahead and push to stable.

IIRC, we agreed to no longer push new features to the stable branch. We should mainly focus on finishing features and changes that have been added during the Octave 7 development cycle and fixing blockers. Like always, documentation changes can also go on stable.
Additionally, bug fixes are ok to go on stable. Sometimes it is hard to draw a line between what is a bug fix and what is a new feature. I'd lean towards the latter in this case. But ultimately, there are no hard rules.

Markus M├╝tzel <mmuetzel>
Project Member
Thu 02 Dec 2021 01:46:14 AM UTC, comment #5: 

refreshed patch to current stable branch. package has no BISTs, but tested against latest build, no errors.  fixes this bug, bug #56128, and bug #60573.

would like to push to stable if no objections.

(file #52402)

Nicholas Jankowski <nrjank>
Project Member
Wed 12 May 2021 03:29:43 AM UTC, comment #4: 

v3 update

I think this update satisfies this bug, bug #56128, and bug #60573.

on top of the changes in comment #2 and comment #3, this checks at the start of pkg update whether -local or -global was called, and if so limits the scope of the update to the installed local or global files.

tested on stable (6.2.1 - May 14, 2021), as i'm not set up to compile and there were enough changes on default to preclude m-file testing, but there didn't seem to be any changes to relevant sections.  could use a check, though to be sure.

testing by installing an out of date package in both local and global, then running pkg update, pkg update -local, and pkg update -global (with various other options and specifying pkg names), and all behaved as expected.  pkg update -local just checked and updated local files to local, same with global. a regular pkg update behaved as normal.

also made a note to NEWS noting behavior change.

(file #51426)

Nicholas Jankowski <nrjank>
Project Member
Wed 12 May 2021 02:57:53 AM UTC, comment #3: 

v2 update changes to the docstring for the pkg update section.

(file #51425)

Nicholas Jankowski <nrjank>
Project Member
Tue 11 May 2021 09:04:27 PM UTC, comment #2: 

for the first part, passing the global local option, the following seems to be a quick fix:

replacing line 728:

        if (compare_versions (forge_pkg_version, installed_pkg_version, ">"))
          feval (@pkg, "install", "-forge", installed_pkg_name);
        endif

with

        if (compare_versions (forge_pkg_version, installed_pkg_version, ">"))
          options_to_pass = varargin (strncmp (varargin, "-", 1));
          options_to_pass(end+1) = "-forge";
          feval (@pkg, "install", options_to_pass{:}, installed_pkg_name);
        endif

passes all -options to pkg install, making sure the last one is -forge.  I don't think duplicate (two '-forge' options) or 'opposite' options (both -local and -global) are a problem, it will behave just as pkg install does, parsing them in order and the final ones taking preference. 

this just leaves adjusting how it generates the local/global lists. currently if you specify -global, i think it will parse both local and global lists, and do updates to both in global, versus just updating the ones in global. i.e., if I had a local audio and no global one, this would see audio, see in can be updated, and install that update in global, leaving the old one in local and the new one to be ignored in global (since local takes precedence). 

as said before i think more consistent would be for -local/-global to only look at the packages in that location, and only try to update those packages there.  (in the case above, -global wouldn't even check for an audio update because it doesn't exist in -global.  update would just look for updates to what's installed where it's told to look.)

(file #51423)

Nicholas Jankowski <nrjank>
Project Member
Mon 10 May 2021 06:10:58 PM UTC, comment #1: 

note that apart from just passing the -global / local option to install, need to check whether the -global/local option affects the way it generates the package update list to make sure it's consistent with the intent.

pkg update looks for all pkgs and updates them, installing global package updates to the local location if user doesn't have admin privileges.

I think pkg update -global should just look at the list of global packages and update them to the global location, ignoring anything in the current 'local' location. and the same thing should happen with a -local option.  I'm not certain that's what it currently does.

Nicholas Jankowski <nrjank>
Project Member
Mon 10 May 2021 05:59:58 PM UTC, original submission:  

as discussed in bug #59821, pkg update accepts a -global or -local option, but after building the list of packages to update fails to pass the option to 'pkg install <pkgname>' as it works through the list. Thus, no matter whether the -global or -local option was specified, pkg will re-run the check for administrative privileges and the install will take place according to the default based on that check. 

just to verify in octave 6.2.1 (), for a globally installed audio package (on windows) using a user-level octave instance with write access to the global package location:

>> __is_elevated_process__
ans = 0

>> pkg list audio
Package Name  | Version | Installation directory
--------------+---------+-----------------------
       audio  |   2.0.1 | ...\mingw64\share\octave\packages\audio-2.0.1

>> pkg update -global audio
**some install warnings about help text**
For information about changes from previous versions of the audio package, run 'news audio'.

>> pkg list audio
Package Name  | Version | Installation directory
--------------+---------+-----------------------
       audio  |   2.0.2 | C:\Users\nicholas.jankowski\octave\audio-2.0.2

stepping through pkg in debug mode, the option first successfully sets global_install = true.  however, it then just calls pkg install -forge audio, and in that case the _is_elevated_process_ = 0 sets global_install = false for the installation. 

Nicholas Jankowski <nrjank>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #52402:  pkg_update_options_v4.diff added by nrjank (4KiB - application/octet-stream)
file #51426:  pkg_update_options_v3.diff added by nrjank (3KiB - application/octet-stream)
file #51425:  pkg_update_options_v2.diff added by nrjank (1KiB - application/octet-stream - partial fix - allows updates to pass options to install command. with doc changes)
file #51423:  pkg_update_options_v1.diff added by nrjank (929B - application/octet-stream - partial fix - allows updates to pass options to install)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by nrjank
  • -email is unavailable- added by nrjank (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 project members can vote.

     

     

     

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-02-23 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-12-02 nrjank StatusPatch Submitted Ready For Test
    2021-12-02 nrjank Attached File- Added pkg_update_options_v4.diff, #52402
        StatusNone Patch Submitted
    2021-05-12 nrjank Attached File- Added pkg_update_options_v3.diff, #51426
    2021-05-12 nrjank Attached File- Added pkg_update_options_v2.diff, #51425
    2021-05-11 nrjank Attached File- Added pkg_update_options_v1.diff, #51423
        Carbon-Copy- Added mtmiller

    Back to the top


    Powered by Savane 3.9