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

mlterm: unbreak #370949

Merged
merged 4 commits into from
Jan 7, 2025
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkgs/applications/terminal-emulators/mlterm/default.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
args@{
stdenv,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this pattern TBH, I prefer using stdenv'...

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't like it much but this is preferable better to the diff noise caused by replacing all references to stdenv with stdenv' IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like it much but this is preferable better to the diff noise caused by replacing all references to stdenv with stdenv' IMHO.

Isn't it just 1 line? It seems like it should either be this line or the mkDerivation line.

Copy link
Member Author

Choose a reason for hiding this comment

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

stdenv is usually used in multiple places for stdenv.is* guards etc. I didn't check for this particular package; I just apply this pattern because it works everywhere.

gcc13Stdenv,
lib,
fetchFromGitHub,
pkg-config,
Expand Down Expand Up @@ -98,6 +99,7 @@ let
commaSepList = lib.concatStringsSep "," (builtins.attrNames (lib.filterAttrs (n: v: v) attrset));
in
lib.withFeatureAs (commaSepList != "") featureName commaSepList;
stdenv = if args.stdenv.cc.isGNU then args.gcc13Stdenv else args.stdenv;
in
Copy link
Member

Choose a reason for hiding this comment

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

It does not look like anything more elaborate than -Wno-error=incompatible-pointer-types would be required to fix compilation, which is far preferable to pinning the old GCC. However, upstream has already fixed these issues in e.g. arakiken/mlterm@3d38b72 and arakiken/mlterm@821a556. So let’s just apply the patches instead.

stdenv.mkDerivation (finalAttrs: {
pname = "mlterm";
Expand Down
Loading