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

pipewire: 0.3.25 -> 0.3.26 #120833

Merged
merged 3 commits into from Apr 29, 2021
Merged

pipewire: 0.3.25 -> 0.3.26 #120833

merged 3 commits into from Apr 29, 2021

Conversation

talyz
Copy link
Contributor

@talyz talyz commented Apr 27, 2021

Motivation for this change
  • Update pipewire to the latest version. See the changelog for details.
  • Fix the tests
  • Add an update script
Things done
  • Tested manually
  • 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.

@legendofmiracles
Copy link
Contributor

Duplicate of #120748

@legendofmiracles legendofmiracles marked this as a duplicate of #120748 Apr 27, 2021
@hmenke hmenke mentioned this pull request Apr 27, 2021
10 tasks
@hmenke
Copy link
Member

hmenke commented Apr 27, 2021

I suggested to include d393845 in #120748.

@legendofmiracles
Copy link
Contributor

legendofmiracles commented Apr 27, 2021

Since now they're on the other PR, can this be closed?

@talyz
Copy link
Contributor Author

talyz commented Apr 27, 2021

I'd prefer to just rebase this on #120748 and remove the package update, since this now also includes an update script.

@balsoft
Copy link
Member

balsoft commented Apr 27, 2021

I would be OK with any way to get 0.3.26 in nixpkgs, I just really wanted hardware volume to work :)

If you want to, I can close my PR and just merge yours.

@jansol
Copy link
Contributor

jansol commented Apr 27, 2021

Either PR seems fine (I'm still confused about the hash syntax policy but whatever). The update script is a step in the right direction so that would be nice to have. However a more common case than new config files getting added seems to be that the available keys change, so the tests should also compare found files and warn about differences.

@talyz
Copy link
Contributor Author

talyz commented Apr 27, 2021

@balsoft Sure, I'm fine either way :)

@jansol I'm not really sure what you mean by that - since the keys are set by the user directly, there isn't much we can do if the schema changes, right? Am I missing something?

@collares
Copy link
Member

Clarifying the concern: Suppose upstream edits a config file (not adding a new file, just editing an existing one). Ideally, tests would fail if someone updates pipewire manually (without the update script) and forgets to copy the new files to the NixOS module.

@jansol
Copy link
Contributor

jansol commented Apr 27, 2021

Upstream has renamed config keys that are present in the default config files and will likely do that again in the future. This would make an old unmodified default config no longer work the intended way so it's something I'd like to detect. Users' customized configs are their own problem and the tests aren't concerned with those.

@talyz
Copy link
Contributor Author

talyz commented Apr 27, 2021

@jansol Well, the installedTests test should show this, right? It enables a "fully featured" pipewire config and runs the test suite - if the default config is severely broken, it should fail. However, the update script should make sure this doesn't happen as often or at all going forward.

@talyz
Copy link
Contributor Author

talyz commented Apr 29, 2021

Anything more to do here or can we merge?

@jansol jansol mentioned this pull request Apr 29, 2021
5 tasks
@jansol
Copy link
Contributor

jansol commented Apr 29, 2021

Works fine on my end, should be good to go.

@talyz talyz merged commit abecdfe into NixOS:master Apr 29, 2021
@talyz talyz deleted the pipewire-0.3.26 branch April 29, 2021 16:46
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.

None yet

6 participants