xdg-desktop-portal: fix build when doCheck = false

[someplaceguy]

Description of changes

Fixes #275863

cc @jtojnar @pks-t @bobby285271

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 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.

[jtojnar]

This is expected, doCheck disabled running tests but Meson has no idea they will not run. If you want to make doCheck = false work, you should also disable building/configuring tests.

[someplaceguy]

@jtojnar How do I disable building tests?

I know nothing about meson and the meson section in the nixpkgs manual doesn't mention anything about disabling the building of tests.

Would removing the "-Dinstalled-tests=true" flag when doCheck = false do that or is something else needed?

[eclairevoyant]

using lib.optionals on the two meson flags related to tests would be a start, yes.

[someplaceguy]

It turned out that I needed to add another flag instead...

[eclairevoyant]

Builds with the override now, approving.

[eclairevoyant]

Result of nixpkgs-review pr 275867 run on x86_64-linux 1

[eclairevoyant]

@jtojnar mind looking at this? changes were made and the previous review is currently blocking merge

[jtojnar]

Right, this should work but I am still not sure why we would want this.

Disabling tests is outside of regular use case IMO, and if a person is already overriding doCheck, they can just as well pass the appropriate flags, so I do not see the point of increasing the complexity of the expression.

Sorry, I should have been clearer about considering this something that should be done by the consumer.

[someplaceguy]

Right, this should work but I am still not sure why we would want this.

There are at least two good reasons:

1. xdg-desktop-portal's tests are unreliable (search for "timeout" inside the tests/ directory and you will find dozens of matches).

Any test with a timeout is guaranteed to fail if the build machine's load is high enough (which is what happened in my case) or if the machine is slow enough (e.g. when running under an emulator for a different architecture).

Such test failures have nothing to do with the code that is being tested, the problem is in the tests themselves.

2. Cross-compilation disables doCheck.

[Frontear]

Right, this should work but I am still not sure why we would want this.

There are at least two good reasons:

1. xdg-desktop-portal's tests are unreliable (search for "timeout" inside the tests/ directory and you will find dozens of matches).

Any test with a timeout is guaranteed to fail if the build machine's load is high enough (which is what happened in my case) or if the machine is slow enough (e.g. when running under an emulator for a different architecture).

Such test failures have nothing to do with the code that is being tested, the problem is in the tests themselves.

2. Cross-compilation disables doCheck.

I've run into very similar issues myself for large-scale PRs (see #342961 and its history :p), and I think it would honestly be worth considering this a more prevalent issue in Nixpkgs. A lot of builds fail on Hydra constantly for this exact reason as seen in #staging:nixos.org. We need a more standardized resolution to these types of problems, as disabling tests/increasing timeouts isn't a good all-cases fix.