Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support cygwin #11238

Closed
wants to merge 18 commits into from
Closed

Support cygwin #11238

wants to merge 18 commits into from

Conversation

Berrysoft
Copy link
Contributor

@Berrysoft Berrysoft commented Mar 7, 2025

Description

This PR adds support for Cygwin & MSYS2.

  • Cygwin wchar_t is UTF-16.
    • wcwidth is replaced by unicode-width. removed.
    • wcrtomb and mbrtowc are replaced by c32rtomb and mbrtoc32.
  • The target is cross-compiled from Windows.
  • There are some bugs on cygwin, so I have to add some workarounds.

Fixes issue #10956, though it has been closed.

Tracking:

TODO:

  • Link libintl correctly
  • Link libpcre2 correctly

@faho faho added the cygwin label Mar 7, 2025
@faho faho added this to the fish 4.1 milestone Mar 7, 2025
@Berrysoft Berrysoft force-pushed the cygwin-upstream branch 2 times, most recently from 16fbfdb to b26cf23 Compare March 7, 2025 17:43
@ridiculousfish ridiculousfish self-assigned this Mar 7, 2025
Copy link
Member

@faho faho left a comment

Choose a reason for hiding this comment

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

Looks fine by me, I think @ridiculousfish would like to take a look.

Awesome work, btw

@Berrysoft Berrysoft marked this pull request as ready for review March 16, 2025 11:03
@ahaoboy
Copy link
Contributor

ahaoboy commented Mar 25, 2025

Thanks for your excellent work. I tried to test it with the latest version(version 4.1.0-alpha0-g509826571) under msys2, but encountered a strange bug. Every time I type a character, a new line is created. I don't know whether I should open a new issue because cygwin does not officially support it yet.

fish

@Berrysoft
Copy link
Contributor Author

Berrysoft commented Mar 25, 2025

@ahaoboy Do you use oh-my-posh? It might be a bug of it, I think. I haven't dig into the code of oh-my-posh so not that sure.

@ahaoboy
Copy link
Contributor

ahaoboy commented Mar 25, 2025

@ahaoboy Do you use oh-my-posh? It might be a bug of it, I think. I haven't dig into the code of oh-my-posh so not that sure.

I use starship, which works fine on fish 3.7.1

https://packages.msys2.org/base/fish

 starship --version
starship 1.22.1
branch:master
commit_hash:d605196
build_time:2025-01-11 17:09:54 +00:00
build_env:rustc 1.84.0 (9fc6b4312 2025-01-07),stable-x86_64-pc-windows-msvc

@Berrysoft
Copy link
Contributor Author

OK, seems that the problem is a little complex. I'll look into it. You may try to disable starship temporarily and see if the problem is solved.

@faho
Copy link
Member

faho commented Mar 25, 2025

This is an issue where I would suspect the terminal, not the prompt.

Fish 4.0 is more demanding on the terminal, and that can trigger bugs.

This isn't a manifestation I've seen, but I know the most frequent trigger is the kitty keyboard protocol, so try set -Ua fish_features no-keyboard-protocols to turn that off (and restart fish after).

@ahaoboy
Copy link
Contributor

ahaoboy commented Mar 25, 2025

I removed all plugins and configurations, and only added a prompt configuration. The printf function will add a new line every 3 or 4 inputs.

I don't know if there are other possible reasons. It depends on whether other Windows users can reproduce the problem with the same configuration.

# ~/.config/fish/config.fish

set -Ua fish_features no-keyboard-protocols

function fish_prompt
    echo

    # This line of code will cause a newline to be added every 3 to 4 inputs.
    printf '%s@%s %s%s%s > ' $USER $hostname (set_color $fish_color_cwd) (prompt_pwd) (set_color normal)
end

@Berrysoft
Copy link
Contributor Author

Well, I can reproduce such problem with oh-my-posh, both with Windows Terminal and mintty. It might be some misc problems between fish and cygwin api and terminal...

@faho
Copy link
Member

faho commented Mar 25, 2025

set -Ua fish_features no-keyboard-protocols

Don't put that in config.fish. It will cause the universal variable to grow.

It's enough to run it interactively. If you must have it in config.fish, use

contains -- no-keyboard-protocols $fish_features
or set -Ua fish_features no-keyboard-protocols

@Berrysoft
Copy link
Contributor Author

Berrysoft commented Mar 25, 2025

@ahaoboy I can reproduce your problem with your config.

It's not adding a new line in 3 or 4 inputs. It's adding a new line once per second you input.

@faho
Copy link
Member

faho commented Mar 25, 2025

It's adding a new line each second you input.

Terminal timestamps, see

fish-shell/src/screen.rs

Lines 854 to 873 in 360cfdb

fn check_status(&mut self) {
let _ = std::io::stdout().flush();
let _ = std::io::stderr().flush();
if !has_working_tty_timestamps() {
// We can't reliably determine if the terminal has been written to behind our back so we
// just assume that hasn't happened and hope for the best. This is important for multi-line
// prompts to work correctly.
return;
}
if self.mtime_stdout_stderr != mtime_stdout_stderr() {
// Ok, someone has been messing with our screen. We will want to repaint. However, we do not
// know where the cursor is. It is our best bet that we are still on the same line, so we
// move to the beginning of the line, reset the modeled screen contents, and then set the
// modeled cursor y-pos to its earlier value.
let prev_line = self.actual.cursor.y;
self.reset_line(true /* repaint prompt */);
self.actual.cursor.y = prev_line;
}
}

You might want to add Cygwin to has_working_tty_timestamps' exceptions.

@Berrysoft
Copy link
Contributor Author

Thank you, @faho ! I have pushed the change and now the problem should be fixed.

@ahaoboy
Copy link
Contributor

ahaoboy commented Mar 25, 2025

Thank you, @faho ! I have pushed the change and now the problem should be fixed.

Can you update https://github.com/Berrysoft/fish-shell/releases? I also want to test if there are any other issues in starship

@Berrysoft
Copy link
Contributor Author

My custom release has been updated.

@ahaoboy

This comment was marked as resolved.

@Berrysoft

This comment was marked as resolved.

@ahaoboy

This comment was marked as resolved.

@Berrysoft
Copy link
Contributor Author

Seems that poll doesn't return -1 on success any more. I removed the workarounds in fd_monitor.rs.

@krobelus
Copy link
Contributor

krobelus commented Apr 1, 2025

which LLVM tree do I use to build the rust target?
It needs f03b100e9319 ([Cygwin] Fix global variable dll import (#121439), 2025-01-03), right? Maybe I can cherry-pick that one?
Alternatively, are there precompiled versions of the rust target available? I guess src/doc/rustc/src/platform-support/x86_64-pc-cygwin.md says no

@Berrysoft
Copy link
Contributor Author

The patch for LLVM has been merged, so I think the latest release is OK. The latest version of nightly rustc is also OK.

Now there's no pre-built std. You can use -Zbuild-std with nightly toolchain.

@krobelus
Copy link
Contributor

krobelus commented Apr 1, 2025

ah ok, cargo build -Zbuild-std --target=x86_64-pc-cygwin seems to work (on a Linux host, a Windows one can't find #include <paths.h> when compiling src/libc.c).

But it seems we need a new libc release (for libc::statfs), and update the nix crate to be compatible with that.
I wonder if anyone has done the nix changes already.

@Berrysoft
Copy link
Contributor Author

A windows host can do that, and I even wrote a PKGBUILD for MSYS2: https://github.com/Berrysoft/fish-msys2/

But it seems we need a new libc release

Yes, we need a new libc, nix and errno release. Nix will introduce some breaking changes in the next release, so we need to change some code after that.

@krobelus
Copy link
Contributor

krobelus commented Apr 1, 2025

https://github.com/Berrysoft/fish-msys2/

thanks, that's very useful

@krobelus krobelus closed this in ec1c247 Apr 1, 2025
@krobelus
Copy link
Contributor

krobelus commented Apr 1, 2025

merged; I squashed some of your commits (hope that's not a problem) and changed only one line I think (1isize -> 1_isize).
Will add a changelog entry once we pull the requisite dependencies

@Berrysoft Berrysoft deleted the cygwin-upstream branch April 1, 2025 15:57
@ahaoboy
Copy link
Contributor

ahaoboy commented Apr 3, 2025

Will update the document to add Windows installation and compilation steps?

https://github.com/fish-shell/fish-shell?tab=readme-ov-file#windows

@faho
Copy link
Member

faho commented Apr 3, 2025

Once it's available upstream in a reasonable fashion - not as long as it involves installing forks, PRs, patches and special repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants