Skip to content

Conversation

@xclaesse
Copy link
Member

@xclaesse xclaesse commented Oct 6, 2025

No description provided.

@xclaesse xclaesse requested a review from jpakkane as a code owner October 6, 2025 01:47
@xclaesse
Copy link
Member Author

xclaesse commented Oct 6, 2025

@nirbheek Copying this to reduce diff between meson devenv and gst-env.py, with small change to match how it's done for bash in Meson.

@xclaesse
Copy link
Member Author

xclaesse commented Oct 6, 2025

I'm wondering if fish and/or zsh can read bash completion files?

Copy link
Member

@eli-schwartz eli-schwartz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit does several things, so should be several commits.

I think it's fine to support zsh here. I'm opposed to including any code for "fish" though.

@xclaesse
Copy link
Member Author

xclaesse commented Oct 6, 2025

I'm opposed to including any code for "fish" though.

Any specific reason?

@xclaesse
Copy link
Member Author

xclaesse commented Oct 6, 2025

This commit does several things, so should be several commits.

done

@xclaesse xclaesse force-pushed the devenv-fish-zsh branch 2 times, most recently from 3c0f820 to 23ddf83 Compare October 6, 2025 12:46
@xclaesse xclaesse merged commit 202d15d into mesonbuild:master Oct 6, 2025
32 checks passed
@xclaesse xclaesse deleted the devenv-fish-zsh branch October 6, 2025 18:47
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Oct 16, 2025
We don't test shell integration in CI -- as is obvious from the fact
that commit 202d15d added additional
code that, in the testsuite would fatally error.

I don't think it is a good idea to add hacky workarounds for software
nobody understands or uses and isn't tested in CI. Which is a
description that has always fit "fish" like a glove (it is a shell that
doesn't even support being run as a login shell, among numerous other
issues).

In mesonbuild#15086 (review)
I raised concerns about its inclusion, but due to the holiday season was
unable to respond in the narrow window of time between being asked for
my reasons and the PR being merged without review. Still, the zsh
changes are great, let's keep them. Zsh is a widely used shell that is
well understood without hacks.
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Oct 16, 2025
It was proposed to be added, but then questioned and removed in
mesonbuild#15086 (comment)

> I'm not sure this is correct. IIUC, this file is passed as a
> configuration file to bash thus it needs to be written with the system
> encoding, which in Python should be what you get when encoding is not
> specified.

It is insane(ly broken) to use a non-ascii-compatible encoding, and we
exclusively write ascii. We also test in CI that the encoding= argument
is *explicitly* passed, and we do so precisely because python 3.15
changes the meaning of `encoding=None` from "system encoding" to
"utf-8", so in the most absolute sense, it is simply wrong to review an
`encoding=` PR and say to remove the encoding to get the system
encoding.

The bash logic has had that problem forever; the zsh logic only since
the recent PR. Switch both to explicitly require utf-8, which is
ascii-compatible and everyone should use anyway.

Fixes:
```
Traceback (most recent call last):
  File ".../mesonbuild/mesonmain.py", line 193, in run
    return options.run_func(options)
           ~~~~~~~~~~~~~~~~^^^^^^^^^
  File ".../mesonbuild/mdevenv.py", line 226, in run
    tmprc = tempfile.NamedTemporaryFile(mode='w')
  File "/usr/lib/python3.13/tempfile.py", line 574, in NamedTemporaryFile
    encoding = _io.text_encoding(encoding)
EncodingWarning: 'encoding' argument not specified

ERROR: Unhandled python exception

    This is a Meson bug and should be reported!
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants