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

Updates for openSUSE/SUSE #5407

Merged
merged 3 commits into from
Jun 15, 2024
Merged

Updates for openSUSE/SUSE #5407

merged 3 commits into from
Jun 15, 2024

Conversation

nodeg
Copy link
Contributor

@nodeg nodeg commented May 10, 2024

This PR

  • updates the documentation for openSUSE/SUSE (it is always recommended to install a stable package from the official repositories if it is available)
  • updates the Corpr docs

When using Copr on openSUSE distros, you need to specify which repository you want to use, otherwise the command will fail:

❯ sudo dnf copr enable wezfurlong/wezterm-nightly
Enabling a Copr repository. Please note that this repository is not part
of the main distribution, and quality may vary.

The Fedora Project does not exercise any power over the contents of
this repository beyond the rules outlined in the Copr FAQ at
<https://docs.pagure.org/copr.copr/user_documentation.html#what-i-can-build-in-copr>,
and packages are not held to any quality or security level.

Please do not file bug reports about these packages in Fedora
Bugzilla. In case of problems, contact the owner of this repository.

Do you really want to enable copr.fedorainfracloud.org/wezfurlong/wezterm-nightly? [y/N]: y
Error: It wasn't possible to enable this project.
Repository 'epel-20240506-x86_64' does not exist in project 'wezfurlong/wezterm-nightly'.
Available repositories: 'rhel-8-aarch64', 'centos-stream-8-aarch64', 'fedora-40-x86_64', 'fedora-38-aarch64', 'fedora-40-aarch64', 'opensuse-tumbleweed-aarch64', 'fedora-rawhide-x86_64', 'opensuse-leap-15.5-aarch64', 'centos-stream-9-aarch64', 'fedora-rawhide-aarch64', 'rhel-8-x86_64', 'opensuse-tumbleweed-x86_64', 'fedora-39-aarch64', 'rhel-9-aarch64', 'fedora-39-x86_64', 'rhel-9-x86_64', 'centos-stream-9-x86_64', 'opensuse-leap-15.5-x86_64', 'fedora-38-x86_64', 'centos-stream-8-x86_64'

If you want to enable a non-default repository, use the following command:
  'dnf copr enable wezfurlong/wezterm-nightly <repository>'
But note that the installed repo file will likely need a manual modification.

The same with a repository works:

❯ sudo dnf copr enable wezfurlong/wezterm-nightly opensuse-tumbleweed-x86_64
[sudo] password for root:
Enabling a Copr repository. Please note that this repository is not part
of the main distribution, and quality may vary.

The Fedora Project does not exercise any power over the contents of
this repository beyond the rules outlined in the Copr FAQ at
<https://docs.pagure.org/copr.copr/user_documentation.html#what-i-can-build-in-copr>,
and packages are not held to any quality or security level.

Please do not file bug reports about these packages in Fedora
Bugzilla. In case of problems, contact the owner of this repository.

Do you really want to enable copr.fedorainfracloud.org/wezfurlong/wezterm-nightly? [y/N]: y
Repository successfully enabled.
  • removes 2 not needed dependencies for building from source (tested on Tumbleweed, Slowroll and Leap 15.5)
❯ ./get-deps
Loading repository data...
Reading installed packages...
Package 'perl-File-Compare' not found.
Package 'perl-FindBin' not found.
Loading repository data...
Reading installed packages...
(...)

However, the installation from source afterwards was successful.

(...) 
   Compiling wezterm-font v0.1.0 (/home/dgedon/git/wezterm/wezterm-font)
   Compiling window-funcs v0.1.0 (/home/dgedon/git/wezterm/lua-api-crates/window-funcs)
    Finished release [optimized] target(s) in 2m 41s

wezterm on  suse via 🐍 v3.11.9 via 🦀 v1.77.2 took 2m41s 
❯ cargo run --release --bin wezterm -- start
    Finished release [optimized] target(s) in 0.18s
     Running `target/release/wezterm start`

Tests/linting

I ran

rustup component add rustfmt-preview
$ cargo test --all
(...)
failures:
    e2e::agent_forward::no_agent_forward_should_happen_when_disabled
    e2e::agent_forward::ssh_add_should_be_able_to_list_identities_with_agent_forward

test result: FAILED. 37 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.64s

$ cargo fmt --all
Warning: can't set `imports_granularity = Module`, unstable features are only available in nightly channel.
(...)

as mentioned in the contribution guide, but was not able to get the tests to pass, although I did not touch the code of WezTerm itself.

Copy link
Owner

@wez wez left a comment

Choose a reason for hiding this comment

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

Thanks for diving in!

docs/install/linux.md Outdated Show resolved Hide resolved

!!! note
It is recommended that you install via Copr so that it is easiest
to stay up to date as future versions of wezterm are released.
If you want nightly versions of WezTerm, it is recommended to use Copr.
Copy link
Owner

Choose a reason for hiding this comment

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

You may disagree, but from my perspective, I know what is in the copr builds and I don't know what the suse (or any other distro) maintainers may have done to the builds they produce, which makes it a bit easier for me to reason about and provide support if folks are running my builds.

I'd prefer to revert this part of the PR back to what I originally wrote.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted my changes. Since you are the maintainer, I rely on your expertise here. However, when someone reads this note, it may imply to always use the nightly builds instead of the stable releases.
If you are interested in what openSUSE maintainers are doing/modifying in WezTerm on openSUSE, take a look at the packages in the OBS or directly at the .spec file. The interesting part begins at line 105.

docs/install/linux.md Outdated Show resolved Hide resolved
docs/install/linux.md Outdated Show resolved Hide resolved
@nodeg
Copy link
Contributor Author

nodeg commented May 13, 2024

Thank you, Wez, for the quick review and the suggestions. I added your suggestions in a separate commit for an easier review, and also rebased the branch. When the PR gets approved, I can squash the temporary commit into the correct one before you merge it.

@nodeg
Copy link
Contributor Author

nodeg commented May 21, 2024

@wez I did a rebase and would appreciate another review 😉

These 2 dependencies are not needed when building from source. I tested
this for Tumbleweed, Slowroll and Leap 15.5.

Signed-off-by: Dominik Gedon <[email protected]>
- Mention different flavours of openSUSE and how to install WezTerm
  there.
- The recommended way is always via the official repositories. However,
  this is not possible for Leap 15.5 as of now.

Signed-off-by: Dominik Gedon <[email protected]>
@Pi-Cla
Copy link
Contributor

Pi-Cla commented Jun 14, 2024

@wez As another openSUSE user I would also like to see a review on this.

The current SUSE installation instructions still recommends adding the openSUSE:Factory repo which is unsafe and not needed and this PR fixes that

@wez wez merged commit 21d8b6b into wez:main Jun 15, 2024
11 of 13 checks passed
@wez
Copy link
Owner

wez commented Jun 15, 2024

Thanks!

@nodeg nodeg deleted the suse branch June 17, 2024 04:45
saep pushed a commit to saep/wezterm that referenced this pull request Jul 14, 2024
* get-deps: remove not needed deps for openSUSE/SUSE

These 2 dependencies are not needed when building from source. I tested
this for Tumbleweed, Slowroll and Leap 15.5.

Signed-off-by: Dominik Gedon <[email protected]>

* docs: Update docs for openSUSE/SUSE

- Mention different flavours of openSUSE and how to install WezTerm
  there.
- The recommended way is always via the official repositories. However,
  this is not possible for Leap 15.5 as of now.

Signed-off-by: Dominik Gedon <[email protected]>

* TMP: address suggestions from @wez

Signed-off-by: Dominik Gedon <[email protected]>

---------

Signed-off-by: Dominik Gedon <[email protected]>
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.

3 participants