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
Show file tree
Hide file tree
Changes from all commits
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
28 changes: 11 additions & 17 deletions pkgs/tools/inputmethods/uim/default.nix
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
args@{
lib,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you add this?

Copy link
Member Author

Choose a reason for hiding this comment

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

You couldn't refer to the args' stdenv otherwise.

stdenv,
gcc13Stdenv,
fetchFromGitHub,
fetchpatch,
shared-mime-info,
autoconf,
automake,
Expand Down Expand Up @@ -53,16 +53,20 @@ assert withNetworking -> curl != null && openssl != null;
assert withFFI -> libffi != null;
assert withMisc -> libeb != null;

let
stdenv = if args.stdenv.cc.isGNU then args.gcc13Stdenv else args.stdenv;
in
doronbehar marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

You can apply uim/uim@99fd890 instead.


stdenv.mkDerivation rec {
version = "1.8.8";
version = "1.8.9";
pname = "uim";

src = fetchFromGitHub {
owner = "uim";
repo = "uim";
rev = "2c0958c9c505a87e70e344c2192e2e5123c71ea5";
rev = version;
Copy link
Contributor

Choose a reason for hiding this comment

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

rev -> tag

Copy link
Contributor

Choose a reason for hiding this comment

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

rev -> tag

Is this new ever since #368177 ? How come that PR didn't change uim?

Copy link
Member

Choose a reason for hiding this comment

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

Only refs/tags/... were migrated

fetchSubmodules = true;
sha256 = "1hkjxi5r49gcna37m3jvykny5hz9ram4y8a3q7lw4qzr52mz9pdp";
hash = "sha256-OqbtuoV9xPg51BhboP4EtTZA2psd8sUk3l3RfvYtv3w=";
};

nativeBuildInputs =
Expand Down Expand Up @@ -120,18 +124,6 @@ stdenv.mkDerivation rec {

patches = [
./data-hook.patch

# Pull upstream fix for -fno-common toolchains
# https://github.com/uim/libgcroots/pull/4
(fetchpatch {
name = "libgcroots-fno-common.patch";
url = "https://github.com/uim/libgcroots/commit/7e39241344ad0663409e836560ae6b5eb231e1fc.patch";
sha256 = "0iifcl5lk8bvl0cflm47gkymg88aiwzj0gxh2aj3mqlyhvyx78nz";
# Patch comes from git submodule. Relocate as:
# a/include/private/gc_priv.h -> a/sigscheme/libgcroots/include/private/gc_priv.h
stripLen = 1;
extraPrefix = "sigscheme/libgcroots/";
})
];

configureFlags =
Expand Down Expand Up @@ -176,6 +168,8 @@ stdenv.mkDerivation rec {
export XDG_DATA_DIRS="${shared-mime-info}/share"
'';

enableParallelBuilding = false;

dontUseCmakeConfigure = true;

meta = with lib; {
Copy link
Contributor

Choose a reason for hiding this comment

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

passthru.updateScript = gitUpdater { rev-prefix = "v"; };

Expand Down
Loading