Incorrect default LD_LIBRARY_PATH

[Michał Sawicz]

Given the following YAML:

```yaml
name: env-bug
base: core22
version: '0.1'
summary: foo
description: bar
grade: devel
confinement: strict

apps:
  hello-world:
    environment:
      plain: $LD_LIBRARY_PATH
      bracketed: ${LD_LIBRARY_PATH}
      prepended: foo:${LD_LIBRARY_PATH}
      complex: ${LD_LIBRARY_PATH:-Foo}
      snapcrafts: ${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}
    command: echo

parts:
  my-part:
    plugin: nil
    override-build: |
      touch ${CRAFT_PART_INSTALL}/foo
      chmod 555 ${CRAFT_PART_INSTALL}/foo
```

Snapcraft sets the default top-level `environment.LD_LIBRARY_PATH` to:

```yaml
# snap/meta.yaml
environment
  LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}
```

But there's no shell to interpret that expansion:

```shell
$ snap run --shell my-snap-name.hello-world -c "printenv plain"
/var/lib/snapd/lib/gl:/var/lib/snapd/lib/gl32:/var/lib/snapd/void
$ snap run --shell my-snap-name.hello-world -c "printenv bracketed"
/var/lib/snapd/lib/gl:/var/lib/snapd/lib/gl32:/var/lib/snapd/void
$ snap run --shell my-snap-name.hello-world -c "printenv prepended"
foo:/var/lib/snapd/lib/gl:/var/lib/snapd/lib/gl32:/var/lib/snapd/void
$ snap run --shell my-snap-name.hello-world -c "printenv complex"

$ snap run --shell my-snap-name.hello-world -c "printenv snapcrafts"

$
```

As far I can tell, there's only simple string replacement happening.

[Michał Sawicz (saviq)]

This got introduced here:

https://github.com/snapcore/snapcraft/pull/3758

[Callahan Kovacs (mr-cal)]

This is interesting. I can reproduce your same behavior with `snap run --shell`.

The idea of using `$SNAP_LIBRARY_PATH${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}` is not new. For core18|20 snaps, this is set inside a shell script called `snap/command-chain/snapcraft-runner`, which is a shell script and I believe the variable substitution occurs correctly.

(source https://github.com/snapcore/snapcraft/blob/71ed1a3698575d9b6a102a3b70dc497ecf88dca3/snapcraft_legacy/internal/meta/_snap_packaging.py#L587)

For core22, we got rid of the `snapcraft-runner` file, so the same PATH variable now lives in `snap/meta.yaml` (as you mentioned in your post).

Do you think the issue is more about how snapd evaluates environment variables? I don't think core22's approach to setting this variable is wrong per-say, but I'm not sure of a better snapcraft-only solution.

[Callahan Kovacs (mr-cal)]

This is interesting. I can reproduce your same behavior with `snap run --shell`.

The idea of using `$SNAP_LIBRARY_PATH${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}` is not new. For core18|20 snaps, this is set inside a shell script called `snap/command-chain/snapcraft-runner`, which is a shell script and I believe the variable substitution occurs correctly.

(source https://github.com/snapcore/snapcraft/blob/71ed1a3698575d9b6a102a3b70dc497ecf88dca3/snapcraft_legacy/internal/meta/_snap_packaging.py#L587)

For core22, we got rid of the `snapcraft-runner` file, so the same PATH variable now lives in `snap/meta.yaml` (as you mentioned in your post).

Do you think the issue is more about how snapd evaluates environment variables?

I don't think core22's approach to setting this variable is wrong per-say. The `{foo:+bar}` syntax should be posix compliant and I'm not sure of a better snapcraft-only solution for prepending to LD_LIBRARY_PATH.

[Michał Sawicz (saviq)]

> Do you think the issue is more about how snapd evaluates environment variables?
>
> I don't think core22's approach to setting this variable is wrong per-say. The `{foo:+bar}` syntax should be posix compliant and I'm not sure of a better snapcraft-only solution for prepending to LD_LIBRARY_PATH.

Well, yes, _if_ you want to use shell substitutions in meta.yaml, SnapD would need to support that explicitly. AFAICS today it only does simple string replacement on `$NAME` and `${NAME}`, nothing fancier. Other than bringing `snapcraft-runner` script back, it's either Snapcraft should stop using that, or SnapD needs to support it… Or maybe just go for `:${LD_LIBRARY_PATH}`, though that may be problematic if it's empty.

[Sergio Schvezov (sergiusens)]

this should be solved by snapd implementing environment as a list as in LP: #1868460