mapfile and unset: Does this actually work?

mapfile and unset: Does this actually work?

[Bart Schaefer]

Doc says:

mapfile
     This associative array takes as keys the names of files; the
     resulting value is the content of the file.  The value is treated
     identically to any other text coming from a parameter.  The value
     may also be assigned to, in which case the file in question is
     written (whether or not it originally existed); or an element may
     be unset, which will delete the file in question.

However:

% touch removable
% zmodload zsh/mapfile
% : $mapfile[removable] ; unset 'mapfile[removable]'
% ls -l removable
-rw-rw-r-- 1 schaefer schaefer 0 Jan 21 17:17 removable

I tried several variations of this and I can't get mapfile to remove anything.

Looking at the code:

static const struct gsu_hash mapfiles_gsu =
{ hashgetfn, setpmmapfiles, stdunsetfn };

There is an unsetpmmapfile function but it doesn't seem to be used
anywhere.  Even after recompiling with that in place of stdunsetfn
there is some strange behavior:

% zmodload zsh/mapfile
% echo 'text' > removable
% ls -l removable
-rw-rw-r-- 1 schaefer schaefer 5 Jan 21 17:42 removable
% noglob unset mapfile[removable]
% ls -l removable
ls: cannot access 'removable': No such file or directory

(so far so good, but)

% echo 'text' > removable
% () { local +h mapfile; unset 'mapfile[removable]' }
% ls -l removable
-rw-rw-r-- 1 schaefer schaefer 0 Jan 21 17:43 removable
% noglob unset mapfile[removable]
% ls -l removable
-rw-rw-r-- 1 schaefer schaefer 0 Jan 21 17:43 removable

Using "unset" on the local has somehow made that file inaccessible, it
just gets truncated instead of removed.  I think this is because:

% echo ${+mapfile[removable]}
0

The hash element for that file is still present but tagged PM_UNSET?

I started playing with this because I was concerned that having a
local mapfile go out of scope would cause files to be removed, but
even with unsetpmmapfile that fortunately doesn't occur.

Re: mapfile and unset: Does this actually work?

[Bart Schaefer]

On Sun, Jan 21, 2024 at 6:05 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> Looking at the code:
>
> static const struct gsu_hash mapfiles_gsu =
> { hashgetfn, setpmmapfiles, stdunsetfn };
>
> There is an unsetpmmapfile function but it doesn't seem to be used

Oops, found the use:

static const struct gsu_scalar mapfile_gsu =
{ strgetfn, setpmmapfile, unsetpmmapfile };

static struct paramdef partab[] = {
    SPECIALPMDEF("mapfile", 0, &mapfiles_gsu, getpmmapfile, scanpmmapfile)
};

Naming (singular vs plural) is a bit confusing here.

Re: mapfile and unset: Does this actually work?

[Bart Schaefer]

If we're contemplating a release, does anyone else want to eyeball this?

On Sun, Jan 21, 2024 at 6:05 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> Doc says:
>
> mapfile
>      This associative array takes as keys the names of files; the
>      resulting value is the content of the file.  The value is treated
>      identically to any other text coming from a parameter.  The value
>      may also be assigned to, in which case the file in question is
>      written (whether or not it originally existed); or an element may
>      be unset, which will delete the file in question.
>
> However:
>
> % touch removable
> % zmodload zsh/mapfile
> % : $mapfile[removable] ; unset 'mapfile[removable]'
> % ls -l removable
> -rw-rw-r-- 1 schaefer schaefer 0 Jan 21 17:17 removable
>
> I tried several variations of this and I can't get mapfile to remove anything.

I still haven't worked out what's going on here.

Re: mapfile and unset: Does this actually work?

[Mark J. Reed]

[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]

FWIW this seems to be a regression introduced in your development branch.
Using the 5.9 release, *unset 'mapfile[removable]'*  does indeed delete the
file. I tested on macOS, Windows+msys2, and Ubuntu.


On Sat, Feb 3, 2024 at 11:25 PM Bart Schaefer <schaefer@brasslantern.com>
wrote:

> If we're contemplating a release, does anyone else want to eyeball this?
>
> On Sun, Jan 21, 2024 at 6:05 PM Bart Schaefer <schaefer@brasslantern.com>
> wrote:
> >
> > Doc says:
> >
> > mapfile
> >      This associative array takes as keys the names of files; the
> >      resulting value is the content of the file.  The value is treated
> >      identically to any other text coming from a parameter.  The value
> >      may also be assigned to, in which case the file in question is
> >      written (whether or not it originally existed); or an element may
> >      be unset, which will delete the file in question.
> >
> > However:
> >
> > % touch removable
> > % zmodload zsh/mapfile
> > % : $mapfile[removable] ; unset 'mapfile[removable]'
> > % ls -l removable
> > -rw-rw-r-- 1 schaefer schaefer 0 Jan 21 17:17 removable
> >
> > I tried several variations of this and I can't get mapfile to remove
> anything.
>
> I still haven't worked out what's going on here.
>
>

-- 
Mark J. Reed <markjreed@gmail.com>

[-- Attachment #2: Type: text/html, Size: 2087 bytes --]

Re: mapfile and unset: Does this actually work?

[Mark J. Reed]

[-- Attachment #1: Type: text/plain, Size: 1704 bytes --]

I take that back. If the file is nonempty, then the file is deleted. If the
file is empty, then it's not. So it seems to be an conflation of unset and
empty in the mapfile logic.

On Sun, Feb 4, 2024 at 12:57 PM Mark J. Reed <markjreed@gmail.com> wrote:

> FWIW this seems to be a regression introduced in your development branch.
> Using the 5.9 release, *unset 'mapfile[removable]'*  does indeed delete
> the file. I tested on macOS, Windows+msys2, and Ubuntu.
>
>
> On Sat, Feb 3, 2024 at 11:25 PM Bart Schaefer <schaefer@brasslantern.com>
> wrote:
>
>> If we're contemplating a release, does anyone else want to eyeball this?
>>
>> On Sun, Jan 21, 2024 at 6:05 PM Bart Schaefer <schaefer@brasslantern.com>
>> wrote:
>> >
>> > Doc says:
>> >
>> > mapfile
>> >      This associative array takes as keys the names of files; the
>> >      resulting value is the content of the file.  The value is treated
>> >      identically to any other text coming from a parameter.  The value
>> >      may also be assigned to, in which case the file in question is
>> >      written (whether or not it originally existed); or an element may
>> >      be unset, which will delete the file in question.
>> >
>> > However:
>> >
>> > % touch removable
>> > % zmodload zsh/mapfile
>> > % : $mapfile[removable] ; unset 'mapfile[removable]'
>> > % ls -l removable
>> > -rw-rw-r-- 1 schaefer schaefer 0 Jan 21 17:17 removable
>> >
>> > I tried several variations of this and I can't get mapfile to remove
>> anything.
>>
>> I still haven't worked out what's going on here.
>>
>>
>
> --
> Mark J. Reed <markjreed@gmail.com>
>


-- 
Mark J. Reed <markjreed@gmail.com>

[-- Attachment #2: Type: text/html, Size: 2871 bytes --]

Re: mapfile and unset: Does this actually work?

[Bart Schaefer]

On Sun, Feb 4, 2024 at 10:04 AM Mark J. Reed <markjreed@gmail.com> wrote:
>
> I take that back. If the file is nonempty, then the file is deleted. If the file is empty, then it's not. So it seems to be an conflation of unset and empty in the mapfile logic.

Aha.  In the "get" method from the gsu struct:

    /* Set u.str to contents of file given by name */
    if ((contents = get_contents(pm->node.nam)))
        pm->u.str = contents;
    else {
        pm->u.str = "";
        pm->node.flags |= PM_UNSET;
    }

So an empty file always appears to [already] be unset, e.g.

 % touch empty
 % zmodload zsh/mapfile
 % print ${+mapfile[empty]}
 0

This in turn appears to be because get_contents() doesn't distinguish
a file it can't open from one whose contents it can't read.  More
specifically, mmap() on an empty file returns -1.

The unset logic in mapfile.c is never called because the parameter is
already marked unset.

Perhaps this is more suited for the BUGS file than for an immediate repair.