bugGNU Octave - Bugs: bug #62681, pkg test: Tests outside of the...

 
 

bug #62681: pkg test: Tests outside of the specified package are being run

Submitter:  alexvong1995 AT protonmail * com <alexvong1995>
Submitted:  Tue 28 Jun 2022 10:05:10 PM UTC
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 7.1.0 Operating System:  * Any
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Tue 19 Jul 2022 07:18:57 AM UTC, comment #19: 

Thanks for testing again.

Closing as fixed.

Markus Mützel <mmuetzel>
Project Member
Mon 18 Jul 2022 06:28:53 PM UTC, comment #18: 

Thanks for the prompt fix.
Verified on my Win10 systems with Octave-8.0.0. Also works on Linux (but we knew that).

I suppose this bug can be closed ?

Philip Nienhuis <philipnienhuis>
Project Member
Mon 18 Jul 2022 03:32:03 PM UTC, comment #17: 

Thank you for checking.
I pushed this additional change to the stable branch that will hopefully fix this regression:
https://hg.savannah.gnu.org/hgweb/octave/rev/07e52e612ea2

Markus Mützel <mmuetzel>
Project Member
Sun 17 Jul 2022 07:00:13 PM UTC, comment #16: 

Since patch cf5f46b2e052, I have a few issues on Windows. "pkg list" just shows a package list with package names, without the path. Furthermore, loading packages doesn't work anymore.

I tracked it down to cset cf5f46b2e052, where in private function expand_rel_path.m, regexprep stumbles over the Windows backslash path separators. That is, if I debug it, I get:

>> pkg list
## stepping into subfunctions until expand_rel_path L.37 ...
debug> loc
loc = C:\Programs\Octave\Octave-8.0.0_20220714D\mingw64
debug> pkg_list{i}.dir
ans = __OH__/share/octave/packages/optim-1.6.2
debug> regexprep (pkg_list{i}.dir, "^__OH__", loc)
ans = C:ProgramsOctaveOctave-8.0.0_20220714Dmingw64/share/octave/packages/optim-1.6.2
debug>

That string "C:ProgramsOctaveOctave-8.0.0_20220714Dmingw64/share/octave/packages/optim-1.6.2" doesn't look right to me ...

(tested w. 7.1.91 from July 16 & a recent 8.0.0)

Philip Nienhuis <philipnienhuis>
Project Member
Fri 08 Jul 2022 11:06:30 AM UTC, comment #15: 

I pushed this change to create the `prefix` and `arch_prefix` directories independently from the build rules of the package that is about to be installed:
https://hg.savannah.gnu.org/hgweb/octave/rev/65c4d98352d3

That should be fixing the underlying issue that we figured out with your help. (Much appreciate it.)

Additionally, I pushed a slightly different patch from the change you proposed in comment #1 here:
https://hg.savannah.gnu.org/hgweb/octave/rev/cf5f46b2e052

That should also work correctly if "__OH__" happens to be an actual part of the path of those prefixes (and is not the replacement prefix).

Marking as ready for test.

Markus Mützel <mmuetzel>
Project Member
Wed 06 Jul 2022 07:13:25 PM UTC, comment #14: 

>  because the "symbolic" package doesn't install any .oct files. (Is that true?)


That is correct.  Its a "noarch" package in Fedora-speak.

Colin Macdonald <cbm>
Wed 06 Jul 2022 06:59:34 PM UTC, comment #13: 

@alex I suspect (but not sure) this is b/c after you install packages as root (as in the "oc" container in the symbolic pkg CI) then the "packages" dir is created.  But not confirmed.

@markus sorry I'm no docker expert!  I Fedora, I use "podman" instead which does not require root.  On Ubuntu, I believe one either does "sudo" or adds your account to some "docker" group.

There are further complications about whether one is root inside the container...  See the "oc" container that @alex refers to, which is created here:
https://github.com/cbm755/octsympy/blob/main/.github/workflows/main.yml#L129

I'll see if I can poke at this a bit too but we may need to loop in the "gnuoctave/octave" container maintainer(s).

Colin Macdonald <cbm>
Wed 06 Jul 2022 05:05:00 PM UTC, comment #12: 

Thanks.

I think we are getting closer.
`canonicalize_file_name` is called on these paths in `pkg/private/make_rel_paths.m`. If I understand Philip's comments in bug #51632 correctly, we can't just drop that call.

`canonicalize_file_name` returns `[]` for non-existent files/folders.

Afaict, compiled .oct files would be installed at the `arch_prefix`. It probably doesn't exist when that code is executed because the "symbolic" package doesn't install any .oct files. (Is that true?)

Maybe, we should create these directories during `pkg install` unconditionally, and not rely on the package to create them.

As a workaround, you could probably create the missing directory manually before installing the package in your CI. Something like this might do that:

docker exec oc install -d /usr/lib/octave/packages

I'll look into how to fix that in Octave core.

I'll need to think about that. But it might be that we should still use the change you proposed in comment #1 even if this underlying issue is fixed.

Markus Mützel <mmuetzel>
Project Member
Wed 06 Jul 2022 03:56:12 PM UTC, comment #11: 

> After doing that you can run one-off commands like:

> docker run -it --rm gnuoctave/octave:7.1.0 octave-cli --eval "canonicalize_file_name ('/usr/share/octave/packages')"
> Entered start.sh with args: octave-cli --eval canonicalize_file_name ('/usr/share/octave/packages')
> Executing the command: octave-cli --eval canonicalize_file_name ('/usr/share/octave/packages')
> ans =

> That is, `ans` is empty.  Same for the other command.
>

For some reason, running

docker exec oc octave-cli --eval "canonicalize_file_name ('/usr/share/octave/packages')";
docker exec oc octave-cli --eval "canonicalize_file_name ('/usr/lib/octave/packages')";

in CI seems to give a different result!

ans = /usr/share/octave/packages
error: ignoring const execution_exception& while preparing to exit
ans =
error: ignoring const execution_exception& while preparing to exit

alexvong1995 AT protonmail * com <alexvong1995>
Wed 06 Jul 2022 10:12:46 AM UTC, comment #10: 

Thank you for the instructions.
But to be honest, I'm a bit at a loss when it comes to running those docker images.
I installed docker on Ubuntu 22.04 with `sudo apt-get install docker.io`.
After that, I tried to pull Octave with the first command from comment #9. That didn't work unless I ran it with `sudo`.

It also looks like I need to start docker with `sudo` with whatever command I tried after that. Without `sudo`, it would always fail with something along these lines:

$ docker run -it --rm gnuoctave/octave:7.1.0 octave-cli --eval "geteuid ()"
docker: Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post "http://%2Fvar%2Frun%2Fdocker.sock/v1.24/containers/create": dial unix /var/run/docker.sock: connect: permission denied.
See 'docker run --help'.

I tested the following:

$ sudo docker run -it --rm gnuoctave/octave:7.1.0 octave-cli --eval "geteuid ()"
Entered start.sh with args: octave-cli --eval geteuid ()
Executing the command: octave-cli --eval geteuid ()
ans = 1111
error: ignoring const execution_exception& while preparing to exit

But as you can see, that doesn't return `0` for me. So, I'll probably not be able to reproduce the original issue with this anyway...

I also suspect that the "docker Octave" didn't run as root in comment #9 either. So, the package would be installed at a different location. And the folders in those commands won't be created.

Is there a way how I could replicate the situation for the original issue? (I.e., a "docker Octave" that will report that it runs as root.)

Markus Mützel <mmuetzel>
Project Member
Wed 06 Jul 2022 01:09:39 AM UTC, comment #9: 

Basic instructions, requested in some earlier thread:

docker pull gnuoctave/octave:7.1.0

After doing that you can run one-off commands like:

docker run -it --rm gnuoctave/octave:7.1.0 octave-cli --eval "canonicalize_file_name ('/usr/share/octave/packages')"
Entered start.sh with args: octave-cli --eval canonicalize_file_name ('/usr/share/octave/packages')
Executing the command: octave-cli --eval canonicalize_file_name ('/usr/share/octave/packages')
ans =

That is, `ans` is empty.  Same for the other command.

- - - -

There does not seem to be such a packages directory:

docker run -it --rm gnuoctave/octave:7.1.0 bash

cd /usr/share/octave

/usr/share/octave$ find . | grep packages

./7.1.0/m/pkg/private/list_forge_packages.m
./7.1.0/m/pkg/private/unload_packages.m
./7.1.0/m/pkg/private/load_packages.m
./7.1.0/m/pkg/private/installed_packages.m
./7.1.0/m/pkg/private/load_packages_and_dependencies.m

Colin Macdonald <cbm>
Mon 04 Jul 2022 07:54:13 PM UTC, comment #8: 

Thanks.

IIUC, that should mean that the package `prefix` will be set to `/usr/share/octave/packages`, and the package `arch_prefix` will be set to `/usr/lib/octave/packages`.

Do those look like valid paths in the container?

After installing a package, what do the following commands return?

docker exec oc octave-cli --eval "canonicalize_file_name ('/usr/share/octave/packages')"
docker exec oc octave-cli --eval "canonicalize_file_name ('/usr/lib/octave/packages')"

Markus Mützel <mmuetzel>
Project Member
Mon 04 Jul 2022 03:15:25 PM UTC, comment #7: 

Running

docker exec oc octave-cli --eval "geteuid ()";
docker exec oc octave-cli --eval "OCTAVE_HOME ()";
docker exec oc octave-cli --eval "user_data_dir ()";
docker exec oc octave-cli --eval "__octave_config_info__ ('libdir')"

with Octave 7.1.0 gives

ans = 0
error: ignoring const execution_exception& while preparing to exit
ans = /usr
error: ignoring const execution_exception& while preparing to exit
ans = /home/jovyan/.local/share
error: ignoring const execution_exception& while preparing to exit
ans = /usr/lib
error: ignoring const execution_exception& while preparing to exit

alexvong1995 AT protonmail * com <alexvong1995>
Mon 04 Jul 2022 07:02:32 AM UTC, comment #6: 

I'd need to look into how to setup a "containerized" Octave and how to run that. (Haven't done that before.)
Could you please give a pointer to instructions (preferably step-by-step for dummies) how to do that?

Could you also please check what the following commands return inside a "containerized" Octave (7.1.0 or newer)?

geteuid ()
OCTAVE_HOME ()
user_data_dir ()
__octave_config_info__ ("libdir")

Markus Mützel <mmuetzel>
Project Member
Fri 01 Jul 2022 04:03:03 AM UTC, comment #5: 

comment #4:

> The current main branch of symbolic should do it.  I've seen this most often on our CI/CD runs and when messing around with containers, not sure I've seen it locally yet.
>
> But I think @alexvong1995 has.
>

What I observed is similar to that of @cbm. I've seen the problem only occurring inside containers but not locally.

There're 3 possibilities I could think of.

1. root privilege is used inside containers but not locally
2. install paths inside containers are different from what we use locally
3. environment variables inside containers are different from what we use locally

I've tried dropping root privilege using "sudo -u nobody ..." (actually the correct way to do it in Debian-based distros is to use "setpriv ..." but we're testing only) inside containers but the problem doesn't go away, so maybe we can rule out 1.

alexvong1995 AT protonmail * com <alexvong1995>
Thu 30 Jun 2022 04:26:45 AM UTC, comment #4: 

The current main branch of symbolic should do it.  I've seen this most often on our CI/CD runs and when messing around with containers, not sure I've seen it locally yet.

But I think @alexvong1995 has.

Colin Macdonald <cbm>
Wed 29 Jun 2022 04:50:25 PM UTC, comment #3: 

The patch looks good in principle.
But to make sure that there isn't another error that is causing that `archprefix` is empty(?) for you in the stored local(?) list, I'd like to reproduce the error here. But I was unsuccessful doing that so far with any package I tried.

How do you install the package?

Markus Mützel <mmuetzel>
Project Member
Tue 28 Jun 2022 11:01:36 PM UTC, comment #2: 

Here is the proposed patch:

(file #53381)

alexvong1995 AT protonmail * com <alexvong1995>
Tue 28 Jun 2022 10:20:18 PM UTC, comment #1: 

While I don't know if this is the right fix, changing

    if (strncmpi (pkg_list{i}.dir, "__OH__", 6))
      pkg_list{i}.dir = [ loc pkg_list{i}.dir(7:end) ];
      pkg_list{i}.archprefix = [ loc pkg_list{i}.archprefix(7:end) ];
    endif

to

    if (strncmpi (pkg_list{i}.dir, "__OH__", 6))
      pkg_list{i}.dir = [ loc pkg_list{i}.dir(7:end) ];
    endif
    if (strncmpi (pkg_list{i}.archprefix, "__OH__", 6))
      pkg_list{i}.archprefix = [ loc pkg_list{i}.archprefix(7:end) ];
    endif

seems to fix the issue for "pkg test symbolic" in CI.

alexvong1995 AT protonmail * com <alexvong1995>
Tue 28 Jun 2022 10:05:10 PM UTC, original submission:  

This bug was originally reported in https://github.com/cbm755/octsympy/issues/1142

When running "pkg test symbolic" in CI, tests outside of the symbolic package are being run.

After some debugging, it appears that the m-file https://hg.savannah.gnu.org/hgweb/octave/file/04120d65778a/scripts/pkg/private/expand_rel_paths.m is causing problem.

This part of the code

    if (strncmpi (pkg_list{i}.dir, "__OH__", 6))
      pkg_list{i}.dir = [ loc pkg_list{i}.dir(7:end) ];
      pkg_list{i}.archprefix = [ loc pkg_list{i}.archprefix(7:end) ];

assumes "__OH__" is a prefix of "pkg_list{i}.archprefix" without actually checking it.

In our case, "pkg_list{i}.archprefix" is the empty path, causing "loc" (which is "/usr") to be prepended to the empty path. The end result is that the tests of all m-file inside "/usr" are being run.

alexvong1995 AT protonmail * com <alexvong1995>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #53381:  fixes-62681.patch added by alexvong1995 (1KiB - 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 cbm (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by alexvong1995 (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-07-19 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2022-07-08 mmuetzel Item GroupNone Unexpected Error or Warning
        StatusConfirmed Ready For Test
    2022-07-06 mmuetzel StatusNeed Info Confirmed
    2022-07-04 mmuetzel StatusNone Need Info
    2022-06-28 alexvong1995 Attached File- Added fixes-62681.patch, #53381

    Back to the top


    Powered by Savane 3.9