chatterino2: unstable-2019-05-11 -> 2.1.7 #79073

strager · 2020-02-02T04:18:34Z

Upgrade chatterino2 to the latest release version.

This commit changes the name of the chatterino2 package. Before this commit, the package is named chatterino2-unstable. After this commit, the package is named chatterino2. If a user wants to upgrade, they must first uninstall chatterino2-unstable before installing chatterino2, else there will be a conflict:

$ nix-env -iA nixpkgs.chatterino2
installing 'chatterino2-2.1.7'
building '/nix/store/hxx0vriblvvlkskcnm4rk32d2ghlvkj5-user-environment.drv'...
error: packages '/nix/store/icgg81lkh1z17zbzw7ncrfqnkhlb39x9-chatterino2-2.1.7/Applications/chatterino.app/Contents/Info.plist' and '/nix/store/65wapk4pd5gj8dgvd245fzv4z799cnw8-chatterino2-unstable-2019-05-11/Applications/chatterino.app/Contents/Info.plist' have the same priority 5; use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' to change the priority of one of the conflicting packages (0 being the highest priority)
builder for '/nix/store/hxx0vriblvvlkskcnm4rk32d2ghlvkj5-user-environment.drv' failed with exit code 1
error: build of '/nix/store/hxx0vriblvvlkskcnm4rk32d2ghlvkj5-user-environment.drv' failed
$ nix-env -q | grep chatterino2
chatterino2-unstable-2019-05-11

$ # chatterino2-unstable conflicts with chatterino2; both can't be
$ # installed at the same time. Uninstall chatterino2-unstable, then
$ # try installing chatterino2 again:

$ nix-env -e chatterino2-unstable
uninstalling 'chatterino2-unstable-2019-05-11'
$ nix-env -iA nixpkgs.chatterino2
installing 'chatterino2-2.1.7'
$ nix-env -q | grep chatterino2
chatterino2-2.1.7

Motivation for this change

Things done

doronbehar · 2020-02-23T17:47:27Z

This commit changes the name of the chatterino2 package. Before this commit, the package is named chatterino2-unstable. After this commit, the package is named chatterino2. If a user wants to upgrade, they must first uninstall chatterino2-unstable before installing chatterino2, else there will be a conflict

That's totally reasonable. Unfortunately nix-env doesn't have a way to know reliably which packages need upgrades as it uses the name attribute to know if an update is needed and not the package's attribute.

doronbehar · 2020-02-23T17:48:32Z

~~Please fix the merge conflicts.~~ Any idea why is this marked as Draft?

strager · 2020-02-23T21:39:03Z

Any idea why is this marked as Draft?

This PR depends on #79067. #79067 should merge first, then we can merge #79073.

strager · 2020-02-25T21:53:14Z

This PR is ready to be merged.

cole-h · 2020-02-28T19:39:08Z

Any reason this one should be merged over #77994?

strager · 2020-02-28T19:47:49Z

Any reason this one should be merged over #77994?

I didn't know #77994 exited when I created my PR. =X

It looks like #77994 has some merge conflicts (which should be easy to fix). My PR (#79073) can merge cleanly.

cole-h

I'd like to ask that you format the file (insert newlines, etc) so that it is easier to look at. It doesn't really matter, but comparing the two PRs, I myself would merge the other one solely because it's easier on the eyes.

Do note that I don't have the power to enforce this; it's just personal preference. Feel free to ignore the suggestion.

pkgs/applications/networking/instant-messengers/chatterino2/default.nix

cole-h · 2020-02-28T19:56:59Z

pkgs/applications/networking/instant-messengers/chatterino2/default.nix

  meta = with lib; {
    description = "A chat client for Twitch chat";
    longDescription = ''
      Chatterino is a chat client for Twitch chat. It aims to be an
      improved/extended version of the Twitch web chat. Chatterino 2 is
      the second installment of the Twitch chat client series
      "Chatterino".
-  '';
+    '';
    homepage = "https://github.com/fourtf/chatterino2";


Suggested change

homepage = "https://github.com/fourtf/chatterino2";

homepage = "https://github.com/Chatterino/chatterino2";

pkgs/applications/networking/instant-messengers/chatterino2/default.nix

cole-h · 2020-02-28T20:00:32Z

pkgs/applications/networking/instant-messengers/chatterino2/default.nix

    fetchSubmodules = true;
  };
-  nativeBuildInputs = [ qmake pkgconfig ];
+  nativeBuildInputs = [ qmake pkgconfig wrapQtAppsHook ];
+  patches = lib.optionals stdenv.isDarwin [ ./darwin-frameworks.patch ];


Could you add a comment about why this is necessary, and why it can't be upstreamed?

cole-h · 2020-02-28T20:02:17Z

pkgs/applications/networking/instant-messengers/chatterino2/default.nix


 mkDerivation rec {
  pname = "chatterino2";
-  version = "unstable-2019-05-11";
+  version = "2.1.7";
  src = fetchFromGitHub {
    owner = "fourtf";


Suggested change

owner = "fourtf";

owner = "Chatterino";

strager · 2020-02-28T20:12:09Z

To clarify: the following commits have already been merged and shouldn't be part of this PR:

cdd1c21 chatterino2: fix install on macOS
1221a6b chatterino2: fix: Could not find the Qt platform plugin "cocoa" in ""

This PR only introduces the following commits:

369381e chatterino2: fix formatting of Nix code
c914ffc chatterino2: unstable-2019-05-11 -> 2.1.7

GitHub's UI is misleading about what commits are part of this PR. I tried to fix this PR, but GitHub gave me server errors...

strager · 2020-02-28T20:14:22Z

I managed to fix this PR by changing the base to staging then back to master. On my screen GitHub now correctly shows only the two commits I intend to be part of this PR. Hopefully this clears up confusion.

cole-h · 2020-02-28T20:15:30Z

You probably need to force-push.

Also, I think it would be better to just have the "fix formatting" commit squashed into the actual version bump. The only difference is two spaces...

cole-h

Eval is failing for unrelated reasons (see: #81325), so rest assured it's not your fault :)

pkgs/applications/networking/instant-messengers/chatterino2/default.nix

strager · 2020-02-28T23:35:45Z

wrapQtAppsHook should be specified iff you cannot use the qt-specific deriver: "If you cannot use mkDerivation or mkDerivationWith above, include wrapQtAppsHook in nativeBuildInputs" (emphasis mine).

The docs say "if", not "iff".

This wasn't resolved

wrapQtAppsHook was discussed in PR #79067.

Regardless, the presence or absence of wrapQtAppsHook is unrelated to this PR (#79073). This PR is about upgrading chatterino2, not fixing style [1] or other issues in chatterino2's .nix file.

[1] The whitespace change isn't worth its own PR, so it came along for the ride here.

cole-h · 2020-02-29T01:10:10Z

You're right. I just figured "since you're already here, it might be nice to make the file more readable." I overlooked your conversation about the presence of wrapQtAppsHook and now that I see where you're coming from, it makes sense. Sorry I became so confrontational and tried to force my opinion on you.

After my prevoius review comments get addressed -- leaving a comment why the patch is necessary and changing the repo owner from fourtf to Chatterino -- I can't wait to see this merged and drop my overlay :)

Upgrade chatterino2 to the latest release version. This commit changes the name of the chatterino2 package. Before this commit, the package is named chatterino2-unstable. After this commit, the package is named chatterino2. If a user wants to upgrade, they must first uninstall chatterino2-unstable before installing chatterino2, else there will be a conflict: $ nix-env -iA nixpkgs.chatterino2 installing 'chatterino2-2.1.7' building '/nix/store/hxx0vriblvvlkskcnm4rk32d2ghlvkj5-user-environment.drv'... error: packages '/nix/store/icgg81lkh1z17zbzw7ncrfqnkhlb39x9-chatterino2-2.1.7/Applications/chatterino.app/Contents/Info.plist' and '/nix/store/65wapk4pd5gj8dgvd245fzv4z799cnw8-chatterino2-unstable-2019-05-11/Applications/chatterino.app/Contents/Info.plist' have the same priority 5; use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' to change the priority of one of the conflicting packages (0 being the highest priority) builder for '/nix/store/hxx0vriblvvlkskcnm4rk32d2ghlvkj5-user-environment.drv' failed with exit code 1 error: build of '/nix/store/hxx0vriblvvlkskcnm4rk32d2ghlvkj5-user-environment.drv' failed $ nix-env -q | grep chatterino2 chatterino2-unstable-2019-05-11 $ # chatterino2-unstable conflicts with chatterino2; both can't be $ # installed at the same time. Uninstall chatterino2-unstable, then $ # try installing chatterino2 again: $ nix-env -e chatterino2-unstable uninstalling 'chatterino2-unstable-2019-05-11' $ nix-env -iA nixpkgs.chatterino2 installing 'chatterino2-2.1.7' $ nix-env -q | grep chatterino2 chatterino2-2.1.7

On GitHub, chatterino2 moved from https://github.com/fourtf/chatterino2 to https://github.com/Chatterino/chatterino2. The old URL redirects to the new URL, but update Nixpkgs' source URL and homepage to point to the new URL to reduce confusion.

strager · 2020-05-26T00:26:17Z

@cole-h I found that chatterino2's upstream had a patch for qtkeychain, so I used that patch. Also, I updated the GitHub URL as you requested.

cole-h

LGTM, thanks. Connects to Twitch chat and lets me interact as expected.

[3 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/79073
1 package built:
chatterino2

cole-h · 2020-05-26T02:31:52Z

pkgs/applications/networking/instant-messengers/chatterino2/darwin-frameworks.patch

@@ -0,0 +1,27 @@
+Fix build on macOS.


Slight nit: the comment about the patch should go above the patch in the derivation, not inside the patch itself. The first place people look (or at least, the first place I look) for the reasoning behind a patch is in the derivation itself.

bbigras · 2020-09-12T20:13:29Z

superseded by a11a629

chatterino2: fix formatting of Nix code

369381e

strager changed the title ~~Chatterino2~~ chatterino2: unstable-2019-05-11 -> 2.1.7 Feb 2, 2020

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Feb 2, 2020

doronbehar approved these changes Feb 23, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

doronbehar mentioned this pull request Feb 23, 2020

chatterino2: unstable-2019-05-11 -> unstable-2019-11-02 (with qt fix) #73143

Closed

10 tasks

strager marked this pull request as ready for review February 25, 2020 18:08

cole-h requested changes Feb 28, 2020

View reviewed changes

strager changed the base branch from master to staging February 28, 2020 20:13

strager changed the base branch from staging to master February 28, 2020 20:13

cole-h requested changes Feb 28, 2020

View reviewed changes

pkgs/applications/networking/instant-messengers/chatterino2/default.nix Show resolved Hide resolved

strager added 2 commits May 25, 2020 16:28

strager closed this May 26, 2020

strager force-pushed the chatterino2 branch from c914ffc to 25fb1a0 Compare May 26, 2020 00:24

strager reopened this May 26, 2020

ofborg bot added the 10.rebuild-darwin: 1 label May 26, 2020

ofborg bot added the 10.rebuild-linux: 1 label May 26, 2020

cole-h approved these changes May 26, 2020

View reviewed changes

bbigras closed this Sep 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chatterino2: unstable-2019-05-11 -> 2.1.7 #79073

chatterino2: unstable-2019-05-11 -> 2.1.7 #79073

strager commented Feb 2, 2020

doronbehar commented Feb 23, 2020

This comment has been minimized.

doronbehar commented Feb 23, 2020

strager commented Feb 23, 2020

strager commented Feb 25, 2020

cole-h commented Feb 28, 2020

strager commented Feb 28, 2020

cole-h left a comment

cole-h Feb 28, 2020

cole-h Feb 28, 2020

cole-h Feb 28, 2020

strager commented Feb 28, 2020

strager commented Feb 28, 2020

cole-h commented Feb 28, 2020

cole-h left a comment

strager commented Feb 28, 2020

cole-h commented Feb 29, 2020

strager commented May 26, 2020

cole-h left a comment

cole-h May 26, 2020

bbigras commented Sep 12, 2020

	homepage = "https://github.com/fourtf/chatterino2";
	homepage = "https://github.com/Chatterino/chatterino2";

chatterino2: unstable-2019-05-11 -> 2.1.7 #79073

chatterino2: unstable-2019-05-11 -> 2.1.7 #79073

Conversation

strager commented Feb 2, 2020

Motivation for this change

Things done

doronbehar commented Feb 23, 2020

This comment has been minimized.

doronbehar commented Feb 23, 2020

strager commented Feb 23, 2020

strager commented Feb 25, 2020

cole-h commented Feb 28, 2020

strager commented Feb 28, 2020

cole-h left a comment

Choose a reason for hiding this comment

cole-h Feb 28, 2020

Choose a reason for hiding this comment

cole-h Feb 28, 2020

Choose a reason for hiding this comment

cole-h Feb 28, 2020

Choose a reason for hiding this comment

strager commented Feb 28, 2020

strager commented Feb 28, 2020

cole-h commented Feb 28, 2020

cole-h left a comment

Choose a reason for hiding this comment

strager commented Feb 28, 2020

cole-h commented Feb 29, 2020

strager commented May 26, 2020

cole-h left a comment

Choose a reason for hiding this comment

cole-h May 26, 2020

Choose a reason for hiding this comment

bbigras commented Sep 12, 2020