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

anki: 2.1.15 -> 2.1.40 (fixes #112849) [DRAFT for others to pick up] #112921

Closed
wants to merge 1 commit into from

Conversation

iblech
Copy link
Contributor

@iblech iblech commented Feb 12, 2021

Motivation for this change

#112849

Things done

I tackled some of the required changes, mostly owing to upstream's directory reorganization. However, I then noticed that upstream has switched to a new build system (bazel). Right now I unfortunately don't have the time to deal with the changes required by this switch.

Note that at the moment, this pull request is not fit for merging! The main executable does not start.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 112921 at 9501094a run on x86_64-linux 1

1 package failed to build:
2 packages built:
1 suggestion:
  • warning: unused-argument

    Unused argument: fetchpatch.
    Near pkgs/games/anki/default.nix:7:3:

      |
    7 | , fetchpatch
      |   ^
    

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 112921 at 9501094a run on aarch64-linux 1

1 package failed to build:
2 packages built:
1 suggestion:
  • warning: unused-argument

    Unused argument: fetchpatch.
    Near pkgs/games/anki/default.nix:7:3:

      |
    7 | , fetchpatch
      |   ^
    

@veprbl veprbl linked an issue Feb 12, 2021 that may be closed by this pull request
2 tasks
@SuperSandro2000 SuperSandro2000 marked this pull request as draft February 12, 2021 18:09
@nessdoor
Copy link
Contributor

I'm picking this up. Trying to fight Bazel and its obscurities, atm.

Just to let other people know.

@yuuyins
Copy link
Contributor

yuuyins commented Aug 15, 2021

Any luck? I may try it as well...

@ArdaXi
Copy link
Contributor

ArdaXi commented Aug 15, 2021

For what it's worth, a while back I spent some time getting 2.1.44 working by taking out Bazel altogether. It was incredibly frustrating, and at some point I chose to build some stuff manually and pull other stuff out of the binary build because I just couldn't any more.

I am in no way proud of this, but if anyone wants to look at it, it's in my overlay. This is not a place of honor, etc etc.

@nessdoor
Copy link
Contributor

nessdoor commented Aug 15, 2021

@yuuyins unfortunately I am stuck at a point where Bazel complains about unspecified targets and I really have no idea how to debug the thing without taking a deeper dive into the query language or whatever else.

A quick rundown of my work so far:

  • attempted to build on NixOS outside of Nix evaluation, by direct invocation of Bazel
  • plugged-in Tweag-made rules for getting Nix-supplied binaries
  • replaced CC, Rust, Python, Node and Yarn toolchains with binaries provided through Nix

Now, at this point all issues related to dynamic linking and whatnot seem to be fixed, and the build progresses well for a while until:

error: Error loading target specification: Could not find specification for target "". Use `--print target-list` for a list of built-in targets

ERROR: /home/tlopez/.cache/bazel/_bazel_tlopez/29596eeeaeb1c96a789bf8decf5825e8/external/io_bazel_rules_rust/cargo/cargo_build_script_runner/BUILD:3:13: output 'external/io_bazel_rules_rust/cargo/cargo_build_script_runner/libcargo_build_s\
cript_output_parser--359502323.rlib' was not created
ERROR: /home/tlopez/.cache/bazel/_bazel_tlopez/29596eeeaeb1c96a789bf8decf5825e8/external/io_bazel_rules_rust/cargo/cargo_build_script_runner/BUILD:3:13: Compiling Rust rlib cargo_build_script_output_parser (1 files) failed: not all output\
s were created or valid

and I swear, I have no clue what this means. I don't know if I got lost in something silly, or I have some methodological issue that prevents me from discovering the cause, or I have to blame the fact that I usually work on these things late at night, but I cannot get past this.

I am sorry to say that the last time I worked on this was on August 1st, then other things got in the way and I couldn't dedicate much time to solve the issue. Maybe I should really get back to it.

If you want to give it a try, I can send you the Bazel recipes that I came up with, so that you don't have to go through hell from the beginning. Mind you, I tried to build inside a simple FHS environment as suggested by the NixOS wiki but to no avail. I don't know, maybe it's still a viable approach, but I wanted to get a cleaner solution anyway.

One thing to note and that made me go a little crazy: the folks at Ankitects, for who knows what reason, use custom Bazel rules and a lot of precompiled binaries pulled directly from their own repos. And they are not always a mirror of upstream rules. For example, their Rust ruleset comes with the persistent worker functionality that is their own addition (although their default build rule explicitly disables it). They make extensive use of replacement patterns to substitute upstream rules with their custom Bazel rule repos without changing the name, so it's not super obvious where the custom rules are actually used in stead of the upstream ones.

I hope someone well-versed in Bazel could help. I'll try to resume the efforts, sorry for the hiatus.

@ArdaXi tomorrow I will give a look at your approach. Maybe purging Bazel out of the way is a viable and most sane solution.

P.S. The error above is non-deterministic. If I launch the build multiple times, it errors out in different places.

@ArdaXi
Copy link
Contributor

ArdaXi commented Aug 15, 2021

@nessdoor Your progress looks similar to mine. I also tried the FHS approach, and I'm honestly kinda impressed you made it so far as to get Yarn working. I guess I've just not had the cognitive bandwidth to wrap my head around Bazel enough to use the Tweag rules.

I haven't worked on this since… June, so my memory's a bit fuzzy, but as I recall I only ended up copying the fluent.proto into the repository because it seemed the way it's handled would change in 2.1.45 so figuring out how that's done by hand didn't seem worth the effort.

The main thing that should really be improved (other than my terrible uncommented Bash) is the line

cp -rv ${anki-bin}/bin/aqt_data/web $pp/aqt/data/web

I remember spending a bunch of effort trying to figure out how those files are built, but I was also just trying to solve a problem, and this solved my problem. It's quite clearly unacceptable as a real solution, though.

Oh, since I don't think I actually have a license on that repo yet, let me just state for the record that to the extent possible under law, I've licensed the pkgs/anki subdirectory CC0 and waived all copyright and related or neighboring rights to the pkgs/anki subdirectory. Just in case anyone's concerned, I'll fix that properly later.

@nessdoor
Copy link
Contributor

nessdoor commented Aug 17, 2021

Ok, sorry for the delay, other things got in the way.

@ArdaXi I took a look at your solution. I see that you perform not many compilations, but just copy files around, run some Python UI builders, compile the protobuf stub and that's it. Is it a feature-complete Anki 2.1.44 what you obtain?

Anyway, I don't know the details about aqt/data/web because it's the first time I hear of Sass, I just see that there is an upstream ruleset (again, mirrored by the Anki team) that performs the generation.

Another issue: apparently, they switched to mdbook-toc for compiling the manual, so the old asciidoc recipe is no longer valid. mdbook-toc is, from what I recall, still missing from Nixpkgs, so its merge would be a minor blocker for this PR. But it's a small utility, it wouldn't be too difficult to get the PR under way.

It's crazy the level of complexity that got into this application. Plus, there aren't many Bazel builds in Nixpkgs from which we can take inspiration, so even the integration step Nix-Bazel will be an interesting one.

That being said, I think I will get back to debugging the build right now. I hope to achieve something in the coming days. If not, I'll go hunting for a Bazel enthusiast to ask for help.

@ArdaXi
Copy link
Contributor

ArdaXi commented Aug 17, 2021

@nessdoor Well, it is a Python application at the core still, so there's not a huge amount of compilation involved, no. The Rust section does need it, see rsbridge.nix. You can tell it's the Sass stuff where I gave up...

I can confirm that that code is running on my system and I've been using it daily since then. UI scaling doesn't work very well (in the app settings, Wayland scaling is fine) but beyond that I've had zero issues.

@nessdoor
Copy link
Contributor

If it's maintainable enough and the inputs can be overridden with not too much arcane magic, then once the manual and Sass compilation are fixed maybe you can attempt a PR? I don't think we have a reason to cope with the upstream Bazel build if not for: 1. maintainability 2. philosophy.

@ArdaXi
Copy link
Contributor

ArdaXi commented Aug 17, 2021

Bypassing Bazel like this is bound to create maintainability issues down the line, since we're always going to be one step behind. I'm really not sure it's feasible to try and make it work within Nixpkgs though, at least in the short term.

I'd obviously like to see this get in and I'm not terribly concerned on how, but I'm afraid my available time/energy to figure out the rest of the build is a bit limited. I was just really happy when after hours of work my Anki wasn't blurry any more...

@nessdoor
Copy link
Contributor

nessdoor commented Aug 18, 2021

heavy heavy curses
bazelbuild/rules_rust#64 (comment)

Making progress, now debugging the Qt build

@nessdoor
Copy link
Contributor

nessdoor commented Sep 20, 2021

Final update: it works.

I'll test the thing a little more, clean up the build process, try to port it to the Nixpkgs framework and then open a PR that supersedes this one.

I hope these steps will be less painful.

I am very sorry for the wait.

EDIT:
Just in case I'll be hit by a bus tomorrow, you can find a quick ugly patch for the upstream build recipe under:
/ipfs/QmYhCM6myg2FsgFiK5hKsa8EahwRPQ2wdPXXMJHamDSn17
Warning: a lot of hardcoded Nix store paths that act as scotch tape.

@samm81
Copy link

samm81 commented Nov 27, 2021

@nessdoor thanks for your work on this, I'm trying to access the IPFS hash but it doesn't seem to be on the network (atm anyways) - is there any other way to share this? maybe on github itself?

@nessdoor
Copy link
Contributor

nessdoor commented Nov 27, 2021

@samm81 hello, thank you for notifying me!
I put my node temporarily offline because of power and money conservation, sorry, I didn't think about the patch!
In any case, I put an updated patch at the following gist. It's against the upstream release 1.47.

While I'm at it, a quick update: unfortunately, I was unable to make it run under the system Nix builder. I had to take away the handy recursive Nix invocations, but something broke in the resulting environment.
I was lazily waiting for the Nix release as to have 2.4 available in unstable, so that I could try and run the daemon with the experimental recursive invocation support enabled, but now that I think about it, I can have that already with the current unstable. Will try over the weekend.

Anyway, after applying the patch, be sure to edit the store paths inside nix/rules_nodejs.patch as to reflect the hashes in your Nix store. Not very reproducible, but should get you going. I've been daily-driving the resulting Anki for 2 months now (sigh, I am awful), and I can't see any issue with it.

@samm81
Copy link

samm81 commented Nov 27, 2021

@nessdoor thank you for getting back to me so quickly - I downloaded and applied the patch, but am not sure where the entry point to the build is - `find . | grep .nix' gives:

./nix/node/nixpkgs_node.nix
./nix/rust/rustenv.nix
./nixpkgs-tools.nix

none of which look like entry points

@nessdoor
Copy link
Contributor

nessdoor commented Nov 27, 2021

@samm81 you can use one of the upstream entrypoints (shell scripts in the top directory and under scripts).
Actually, this is a Bazel build, so you need to have Bazel available.
You can see the build commands by reading the upstream entrypoints.

There is no Nix-compliant entry point for the reason I mentioned. Bazel will perform the necessary Nix invocations on its own through the Tweag rules, therefore it needs to be run outside of a Nix builder (for now).

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 19, 2022
@nessdoor
Copy link
Contributor

nessdoor commented Jan 2, 2023

More than a year later, spurred by the discussion happening in #78449, I decided to start investing a little bit of my free time on this, again. The latest versions of Anki have switched build system to a combination of a custom meta-builder written in Rust + Ninja.

For now, I just started writing a Nix expression that goes like this:

{ lib, stdenv, fetchurl
, ninja
, openssl
, pkg-config
, python39
, rustPlatform
, symlinkJoin
}:

let
  pname = "anki";
  version = "2.1.55";

  src = fetchurl {
    url = "https://github.com/ankitects/anki/archive/refs/tags/${version}.tar.gz";
    sha256 = "sha256-H/DMMWZBFSTlf6qauT2hbkOXXKb2KXsx+Y7LSwvfnjI=";
  };

  cargoLock = {
    lockFile = ./Cargo.lock;
    outputHashes = {
      "csv-1.1.6" = "122i931b8bjr0z7rk05axj705gc60rlnx1dfgj48z44mydyvrgf3";
      "hyper-timeout-0.4.1" = "0p5xbz1v9j1vym0xpddnrl93kfj7285svs08h2a2mvc4x9vgwkdc";
      "linkcheck-0.4.1-alpha.0" = "0qp0ljjagzmkiiypdas1i5scwccp09nmsjv4azpjxxynh7r2cahn";
      "pct-str-1.1.0" = "155rvwmzrv18fb09z59n72rwswlyr87fgv5kq3ylm7830y3a9wzb";
      "reqwest-0.11.3" = "0ki3gf34hgq74cfhjbgqr041xdixr5m5amk21l223ixzacp4xk54";
      "tokio-io-timeout-1.1.1" = "19gaqv9gd1spdmlhplyf1krcvfaj3qdrj9nr5lqj5rh1xi3khkm9";
    };
  };

  python = python39;

  # The runner is responsible for running the build
  anki-runner = rustPlatform.buildRustPackage {
    pname = "anki-build-runner";
    inherit version src cargoLock;

    buildAndTestSubdir = "build/runner";

    nativeBuildInputs = [ pkg-config ];
    buildInputs = [ openssl ];

    doCheck = false;
  };

  # The configurator generates the build.ninja
  anki-configurator = rustPlatform.buildRustPackage {
    pname = "anki-build-configurator";
    inherit version src cargoLock;

    buildAndTestSubdir = "build/configure";

    doCheck = false;
  };

  # Compose a derivation for the meta-build system
  anki-build-system = symlinkJoin {
    name = "anki-build-system-${version}";
    paths = [
      anki-runner
      anki-configurator
    ];
  };
in stdenv.mkDerivation {
  inherit pname version src;

  nativeBuildInputs = [ anki-build-system ];

  # Activate optimizations
  RELEASE = "1";

  # Use the Python interpreter from Nixpkgs, otherwise the build system will try
  # to download it
  PYTHON_BINARY = "${python}/bin/python";

  buildPhase = ''
    ${anki-build-system}/bin/runner;
  '';
}

I noticed that by setting certain environment variables, some of the build steps that automatically download stuff are inhibited. I guess our best bet is to generate a sane build.ninja by toying around with the custom meta-builder, and then invoke our Ninja hooks on that.

For now, consider my new efforts even less than a draft. I can't promise I will be able to commit my time to this on a stable basis.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2023
@euank euank mentioned this pull request Mar 16, 2023
11 tasks
@euank
Copy link
Member

euank commented Mar 16, 2023

I picked this up over in #221229

Thank you for the initial start and thoughts, @nessdoor, I started from that for my PR, and it was helpful!

@Atemu Atemu closed this Mar 16, 2023
@nessdoor
Copy link
Contributor

Thank you for leading this story to an end (hopefully), glad to have been of any help. Really looking forward to a fresh, sleek, source-built Anki package, finally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anki 2.1.40
9 participants