Skip to content

i3status-rust: make notmuch optional#387062

Merged
drupol merged 2 commits intoNixOS:masterfrom
just1602:i3status-rs-optional-notmuch
Mar 5, 2025
Merged

i3status-rust: make notmuch optional#387062
drupol merged 2 commits intoNixOS:masterfrom
just1602:i3status-rs-optional-notmuch

Conversation

@just1602
Copy link
Contributor

@just1602 just1602 commented Mar 4, 2025

Hello,

With notmuch recently failling to build, it makes i3status-rust failling to build too. This PR make it possible to disable notmuch support if desire.

I'm currently using this patch as an overlay on my config and it works perfectly fine.

My editor has nixd configured for lsp and it format on save, so it format the file using nixfmt-rfc-style, I assume it is ok, but if it's not I'll revert those modification and only add the bits to make notmuch optional.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 4, 2025
@nix-owners nix-owners bot requested a review from globin March 4, 2025 18:22
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

With notmuch recently failling to build, it makes i3status-rust failling to build too.

I guess this PR is not going to fix that issue since withNotMuch is enabled by default. Right?

Tip for the next time: make sure to correctly separate your commits, especially when there are reformatting commits like in this PR. Without that, it makes the review much harder.

@just1602
Copy link
Contributor Author

just1602 commented Mar 4, 2025

@drupol as I said in the other PR, no it doesn't disable it by default because I didn't want to break the current behavior, but I want to make it possible for people to override the attribute and disable it if needed.

Otherwise, a lot of people who use notmuch will upgrade and get their mail setup broken without any notice, that wouldn't make sense.

Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

The diff LGTM, though I would have preferred separate commits.

Note: This PR does not fix i3status-rust; it only adds a flag to disable notmuch, which is the underlying issue preventing i3status-rust from building correctly.

While this is merely a workaround rather than a true fix, I’m okay with this change since the default flag value remains unchanged, meaning the real issue still needs to be addressed. That said, having the option to disable notmuch via a flag is a useful addition.

Edit: Fixed in #386681

@drupol
Copy link
Contributor

drupol commented Mar 5, 2025

@just1602 I see that the CI succeeded to build i3status-rust correctly on x86_64-linux and aarch64-linux.

In your original message, you mention:

With notmuch recently failling to build, it makes i3status-rust failling to build too.

Can you please let us know more? Which arch/platform is failing for you?

@mhutter
Copy link
Contributor

mhutter commented Mar 5, 2025

@drupol see #386681

@just1602
Copy link
Contributor Author

just1602 commented Mar 5, 2025

@drupol before #386738 got merged, every package that were relying on notmuch failed to build. This patch make it possible to disable notmuch support for people who don't use it or want to override neomutt attrs to make it build anyways even if notmuch doesn't build again.

@mhutter beat me

@drupol drupol requested a review from GaetanLepage March 5, 2025 15:41
@GaetanLepage
Copy link
Contributor

Please, definitely split the patch. Formatting should happen in a different commit.

@just1602 just1602 force-pushed the i3status-rs-optional-notmuch branch from e262077 to 0c5ca49 Compare March 5, 2025 16:51
@drupol drupol merged commit a7b961c into NixOS:master Mar 5, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants