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

Nix updates 2.3 #3258

Closed
wants to merge 54 commits into from
Closed

Nix updates 2.3 #3258

wants to merge 54 commits into from

Conversation

poelzi
Copy link
Contributor

@poelzi poelzi commented Nov 3, 2020

These is a backport of #3174 to 2.3.

It fixes all issues I found so far in the derivate.
This will allow us to deliver up to date derivates through cachix to the enduser.
This branch now has working travis for nix and runs all tests.

@github-actions github-actions bot added the build label Nov 3, 2020
@poelzi
Copy link
Contributor Author

poelzi commented Nov 6, 2020

@haslersn wanna review ?
I'm using this version now for quite some time and it works very nicely. I added nix ci that run the tests and will publish to cachix.
This way you will be able to test a branch without compiling :)

I would like to have this in 2.3 soon, so we can official builds.

@poelzi poelzi force-pushed the nix-updates-2.3 branch 2 times, most recently from 8d798cd to bb5991f Compare November 14, 2020 04:26
@ronso0 ronso0 added this to the 2.3.0 milestone Nov 16, 2020
@poelzi poelzi force-pushed the nix-updates-2.3 branch 3 times, most recently from 19ceb4d to 8e636ef Compare November 24, 2020 18:36
poelzi and others added 20 commits November 24, 2020 21:36
* Fixes lv2 wrapper so plugins show up again
* Remove old binaries so makeWrapper does not create useless backups
* Autodetect cmake buildtype and set type
* Fix locale problems on none nix hosts
* Remove useless imports and use "with" where possible
* format sources with nixfmt
* ingnore result* for sources
* Allow cmake flags to be passed
* wrap mixxx-test for running in ci environment
* set QT_PLUGIN_PATH in case we run tests in a CI without proper paths
push packages to cachix
Correctly report error codes. Run tests in travis dev shell

don't fail if cache copy fails
Including the setup hook script does not work and only causes problems.
Simply use nix-shell to run the wrapper in a reliable way.

Add nodjs for eslint
Inform the user that this combination might break and how to
mitigate it.
LLVM < 10.0.2 with glibc >= 2.31 don't link well with fast-math
active. Add option to disable this optimization alone
@Holzhaus
Copy link
Member

Holzhaus commented May 13, 2021

Just for my understanding: Why do we need to pull the NixOS artifacts into the Mixxx source repo and extend our CI builds instead of hosting the configuration files in nixpkgs? There is even a dedicated build system for NixOS packages.

I think nixpgs is for stable packages, but this config is for development versions and building from source. I think https://github.com/nix-community/NUR would be the proper repo for that.

@Holzhaus
Copy link
Member

How do we continue here? We shouldn't tag 2.3.0 with a broken nixOS config. I think we have 4 alternatives:

  1. Merge this PR including CI builds
  2. Merge this PR excluding CI builds
  3. Merge Fixes for shell.nix #3795 instead of this PR
  4. Merge neither and remove the nixOS config from our source tree

Opinions?

@JoergAtGithub
Copy link
Member

I'm in favour of #1, because the CI build with Clang compiler ensures compatibility, not only for NIX.

@Holzhaus
Copy link
Member

Holzhaus commented May 15, 2021

I'm in favour of #1, because the CI build with Clang compiler ensures compatibility, not only for NIX.

We already ensure clang compatibility with the clang-tidy, clazy and macOS CI builds.

@uklotzde
Copy link
Contributor

The CI builds are not needed. NixOS users will notice any breakage when building locally and could then file a PR. Why should we maintain CI builds that affect only very few users, are unmaintainable for most of us, and are not needed for any official release? We also don't host CI builds for other Linux distributions.

@uklotzde
Copy link
Contributor

I'm in favour of #1, because the CI build with Clang compiler ensures compatibility, not only for NIX.

We already ensure clang compatibility with the clang-tidy, clazy and macOS CI builds.

Correct, we already have one CI build for each compiler suite (GCC, Clang, MSVC). We cannot test all possible combinations including different versions, platforms, CPU architectures, ...

@Holzhaus
Copy link
Member

Holzhaus commented May 15, 2021

I'd prefer 4, but I'm also okay with 3 or 2 if most others disagree. If we chose 1, all PRs with failing nixOS builds are essentially blocked until @poelzi reviews them and contributes a fix. Or we ignore if the nixOS CI fails, which makes them kinda pointless.

@daschuer
Copy link
Member

I vote for 2 or 3. If we decide for 2, I would prefer to cut out unrelated changes into single PRs.

@uklotzde uklotzde modified the milestones: 2.3.1, 2.3.2 Sep 7, 2021
@poelzi poelzi mentioned this pull request Sep 14, 2021
@github-actions
Copy link

github-actions bot commented Dec 7, 2021

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Dec 7, 2021
@daschuer daschuer removed this from the 2.3.2 milestone Jan 15, 2022
@daschuer
Copy link
Member

Can we close this stale PR now? Our main build tool is cmake and we don't have the power to maintain another build script of 397 lines.
See the discussion in #3795 (comment) for the option striping it down to an environment setup only tool.

@github-actions github-actions bot removed the stale Stale issues that haven't been updated for a long time. label Jan 19, 2022
@github-actions
Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Apr 20, 2022
@cr7pt0gr4ph7
Copy link
Contributor

Currently going through old & stale PRs in an effort to reduce the number of open PRs.
I'd argue that if this build script update wasn't relevant two years ago, it very much isn't relevant now, and can be closed.

@daschuer
Copy link
Member

daschuer commented Jun 7, 2024

Yes, let's close this and than also decide for
#3795
as well.

@daschuer daschuer closed this Jun 7, 2024
@cr7pt0gr4ph7
Copy link
Contributor

Yes, let's close this and than also decide for #3795 as well.

#4632 would also be a candidate for closing then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build stale Stale issues that haven't been updated for a long time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants