Not really sure what the CI test errors are supposed to mean...

No worries, the CI test errors were unrelated and should have been fixed in master. Can you please rebase on top of master?

I think just starting a new Wayland compositor process within the Vim test instance would be the best way for the tests. So Niri will be the best fit since it supports both data control protocols.

@chrisbra Would it be possible for the CI tests to have a Wayland compositor installed? Current tests use Niri, but requires a really new version (v25.02), not sure if that is in the repos yet.

Should be fine to add, but probably depends on how much effort it is to install Niri (v25.02).

@chrisbra @64-bitman any opinion on this? https://github.com/vim/vim/pull/17097/files#r2146998586

Oh yeah, missed that one and definitely agree on the users expectations here. Thanks.

@chrisbra
#17097 (comment)

The PR includes the artifacts from wayland-scanner, but wouldn't it be better to run wayland-scanner at build time?

That is what I was thinking, maybe @chrisbra could give his opinion?

The PR includes the artifacts from wayland-scanner, but wouldn't it be better to run wayland-scanner at build time?

Ah yes, So how about applying this patch on top? @64-bitman what do you think?

The PR includes the artifacts from wayland-scanner, but wouldn't it be better to run wayland-scanner at build time?

Ah yes, So how about applying this patch on top? @64-bitman what do you think?

@chrisbra Looks good, I've added some changes, can you check? I'll squash it back into one commit then. Thanks

Hm, I am a bit worried that the new Makefile now is GNU specific.

Hm, I am a bit worried that the new Makefile now is GNU specific.

Any hints on any GNU specific stuff in the Makefile?

I think those things are GNU specific:

• $(shell ..)
• ifndef SCANNER
• $(shell ...)
• .PHONY

If you are using debian/ubuntu, you may install bmake and try to run the makefile with it.

I need to check the details with a BSD machine, don't have one right handy right now.

I think those things are GNU specific:

• $(shell ..)
• ifndef SCANNER
• $(shell ...)
• .PHONY

If you are using debian/ubuntu, you may install bmake and try to run the makefile with it.

I need to check the details with a BSD machine, don't have one right handy right now.

I've hopefully removed all the GNU specific stuff. Makefile works with bmake for me.

Thanks, any suggestions on the documentation update @dvogel ?

Thanks, any suggestions on the documentation update @dvogel ?

Sorry but I haven't had a chance to look back at this and I likely won't any time soon. Don't hold this up on my account. If my concerns turn out to be warranted we can address them then.

@chrisbra Anything holding back from this being merged?

I was just looking at this again and noticed it triggers ASAN failures. I re-triggered CI, but did not help. Any idea why this would cause ASAN failure?

I was just looking at this again and noticed it triggers ASAN failures. I re-triggered CI, but did not help. Any idea why this would cause ASAN failure?

I don't know. None of the failures seem to be related to what this PR has touched...

CI is pretty unreliable recently. Anyhow, I just disabled the failing Turkish locale test, so that one we can ignore for now. But we can clearly see here:

CONFOPT: --enable-perlinterp=yes --enable-pythoninterp=no --enable-python3interp=yes --enable-rubyinterp=yes --enable-luainterp=yes --enable-tclinterp=yes 
    SANITIZER_CFLAGS: -g -O0 -DABORT_ON_INTERNAL_ERROR -DEXITFREE -fsanitize-recover=all -fsanitize=address -fsanitize=undefined -fno-omit-frame-pointer
    ASAN_OPTIONS: print_stacktrace=1:log_path=/home/runner/work/vim/vim/logs/asan
    UBSAN_OPTIONS: print_stacktrace=1:log_path=/home/runner/work/vim/vim/logs/ubsan
    LSAN_OPTIONS: suppressions=/home/runner/work/vim/vim/src/testdir/lsan-suppress.txt
    CFLAGS: 
    NO_AT_BRIDGE: 1
/home/runner/work/vim/vim/logs/ubsan_test_wayland.28006

that test_wayland causes undefined behaviour.
Can you reproduce this failure with those leak flags? Some of the reported leaks happen right before exiting Vim. I am also wondering, why those allocations happen after calling mch_exit() which then causes some options to be set again?

CI is pretty unreliable recently. Anyhow, I just disabled the failing Turkish locale test, so that one we can ignore for now. But we can clearly see here:

CONFOPT: --enable-perlinterp=yes --enable-pythoninterp=no --enable-python3interp=yes --enable-rubyinterp=yes --enable-luainterp=yes --enable-tclinterp=yes 
    SANITIZER_CFLAGS: -g -O0 -DABORT_ON_INTERNAL_ERROR -DEXITFREE -fsanitize-recover=all -fsanitize=address -fsanitize=undefined -fno-omit-frame-pointer
    ASAN_OPTIONS: print_stacktrace=1:log_path=/home/runner/work/vim/vim/logs/asan
    UBSAN_OPTIONS: print_stacktrace=1:log_path=/home/runner/work/vim/vim/logs/ubsan
    LSAN_OPTIONS: suppressions=/home/runner/work/vim/vim/src/testdir/lsan-suppress.txt
    CFLAGS: 
    NO_AT_BRIDGE: 1
/home/runner/work/vim/vim/logs/ubsan_test_wayland.28006

that test_wayland causes undefined behaviour. Can you reproduce this failure with those leak flags? Some of the reported leaks happen right before exiting Vim. I am also wondering, why those allocations happen after calling mch_exit() which then causes some options to be set again?

I can't reproduce with those leak flags, unfortunately. I only get some X11 related leaks which I'm pretty sure have always been like that?

Neither can I seem to reproduce it. Seems like some false positives. Let's try to skip this test with in CI when ASAN is detected and clang compiler is used.

Okay on a different machine, I can reproduce it and I think I found it. It seems to happen in Test_wayland_startup() because the remote Vim is not correctly shutdown. I'll fix this while merging. Please watch out for further issues (potentially coverity may also complain once this is merged), I hope you will be able to have a look in those cases.

Thanks all for making vim better!

Thank you @64-bitman and everyone else!

Does this mean I can remove this from my config once the version with this change gets in to my distro's (arch) repository?

Also why was this closed instead of being merged?

It's being closed, because it has been merged using the git command line.

It's being closed, because it has been merged using the git command line.

Maintainers can often update PRs (docs). So if you want them to appear as merged rather than closed, you can update the PR (git push git@github.com:64-bitman/vim.git wayland_clipboard in this case) before pushing to master and that should do it.

the last time I tried this I got permission denied errors. And even if not, I would not know what I should push to the PR to make it appear as merged here.
In any case, I prefer working with the git command line so I can better review the diff and I for incrementing the patch version I have to do manual work anyhow.

And even if not, I would not know what I should push to the PR to make it appear as merged here.

Just whatever you about to push to master. For a PR to be detected as merged target branch needs to contain the top commit from the PR.

@64-bitman Thank you so much for this!

Unfortunately I can't check it in gnome (I am on KDE since quite a time), but in KDE it works in gvim/vim in st/vim in foot.

Finally, no wl-copy/wl-paste workarounds!

@64-bitman

Coverity complains about insecure temporary file when using tmpfile(). Can we avoid this and use vim_tempname() together with mch_fopen() instead?

@64-bitman

Coverity complains about insecure temporary file when using tmpfile(). Can we avoid this and use vim_tempname() together with mch_fopen() instead?

I mean we just need a file descriptor that can be read and written to, so looks fine to me