user dropped to shell if if boot dataset has mountpoint=legacy

[ofthesun9]

System information

Type	Version/Name
Distribution Name	bookworm
Distribution Version	12
Kernel Version	6.1.0-5-amd64
Architecture	amd64
OpenZFS Version	2.1.9 (debian 2.1.9-2)

Describe the problem you're observing

booting (I am using ZFSBootMenu) a dataset where mountpoint=legacy is not working anymore

The boot cmdline sets root=zfs:rpool/ENCR/MISC/machines/bookworm

As far as I get it, the zfs script provided by zfs-initramfs should be

# Last hail-mary: Hope 'rootmnt' is set!
if [ "$mountpoint" = "legacy" ]; then
          ZFS_CMD="mount.zfs"
mountpoint=""
fi

instead of

# Last hail-mary: Hope 'rootmnt' is set!
mountpoint=""
if [ "$mountpoint" = "legacy" ]; then
           ZFS_CMD="mount.zfs"
fi

This fixed my issue.

Describe how to reproduce the problem

Include any warning/errors/backtraces from the system logs

[rincebrain]

mountpoint=legacy, or mountpoint=none?

because if you have mountpoint=legacy, that change you made shouldn't change you, and if you don't, then you probably mean mountpoint=none, and you want #14473, I believe.

[ofthesun9]

I do have mountpoint=legacy
I am using "debian" pakage 2.1.9-2 which is adding some patches issued after the 2.1.9 release

I think that the commit eb823cb is the cause of my issue.
the line 363 should be after the "if" statement (lines 364 365 366)

[rincebrain]

Ah, I see. It took me a few reads to notice the problem here. Oh bother.

I don't know if your change is correct either, actually, since I'm pretty sure resetting that in both cases was intended, it's just the conditional being broken by doing so that's the problem...so I think moving the set below the if-fi entirely would work too.

@freqlabs, it seems like #14473 (and #14455) are incorrect because you clear mountpoint right before checking it. Is there some reason the if can't be above that? (I can write the patch, obviously, I just figured I'd ask if there was something that breaks that was the reason you moved it to after in the first place...)

[ryao]

@ofthesun9 Embarassingly, this was missed by our eyes (mine included) during review. Please open a PR with your change so that we can get it merged. It looks good to me.