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

Jump to bottom

ebtables: restore legacy commands #81545

Merged
merged 1 commit into from Jan 18, 2021
Merged

ebtables: restore legacy commands #81545

merged 1 commit into from Jan 18, 2021

Conversation

yesbox
Copy link
Contributor

@yesbox yesbox commented Mar 2, 2020 •
edited

ebtables 2.0.11 renamed the ebtables commands from "ebtables*" to "ebtables-legacy-*".

Of course this breaks legacy packages and scripts that depends on the ebtables commands.

The idea behind this upstream change appears to be that ebtables-nft replaces ebtables and distributions should rename either the ebtables-legacy or ebtables-nft commands to provide the ebtables commands.

For nix a better fit is for packages to specify either the ebtables or the ebtables-nft package, while both packages provide the same commands.

This patch restores the ebtables package so it functions again.

  • 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.

@yesbox yesbox mentioned this pull request Mar 2, 2020
Closed
@misuzu
Copy link
Contributor

misuzu commented Mar 4, 2020 •
edited

This should probably be done like this to support both variants and make a switch at some point.
Also related: #81172

@yesbox
Copy link
Contributor Author

yesbox commented Mar 15, 2020

This should probably be done like this to support both variants and make a switch at some point.
Also related: #81172

Perhaps. I don't quite see the purpose. If a package depends on nftables and not ebtables, should it not just replace ebtables with nftables or vice versa? That's the mechanism we have for choosing different packages. Is there a use-case for a package or system depending on both the new nftables commands and the old ebtables commands simultaneously, by both being present and named differently?

@misuzu
Copy link
Contributor

misuzu commented Mar 15, 2020

If a package depends on nftables and not ebtables, should it not just replace ebtables with nftables or vice versa?

It's more of ebtables-nft vs ebtables-legacy - they both have same CLI interface but use different kernel API under the hood.

Is there a use-case for a package or system depending on both the new nftables commands and the old ebtables commands simultaneously, by both being present and named differently?

nftables and ebtables is just a frontends for kernel API's.
nft and ebtables-nft commands can be used at the same time but using different API's concurrently (ebtables-legacy and iptables-nft for example) is a bad idea.

@misuzu
Copy link
Contributor

misuzu commented Mar 15, 2020 •
edited

Disregard my previous comments, ebtables-nft command is included in iptables package.

@yesbox yesbox force-pushed the ebtables-legacy branch 2 times, most recently from 272b893 to 188eb75 Compare May 2, 2020 13:16
@yesbox yesbox requested a review from dtzWill May 2, 2020 13:21
@SuperSandro2000
Copy link
Member

@yesbox please resolve the merge conflict.

@yesbox
ebtables: restore "legacy" commands
1cb447c 
ebtables 2.0.11 renamed the ebtables commands
from "ebtables*" to "ebtables-legacy-*".

Of course this breaks legacy packages and scripts
that depends on the ebtables commands.

The idea behind this upstream change appears to be that
ebtables-nft replaces ebtables and distributions should
rename either the ebtables-legacy or ebtables-nft commands
to provide the ebtables commands.

For nix a better fit is for packages to specify either the
ebtables or the ebtables-nft package, while both packages
provide the same commands.

This patch restores the ebtables package so it functions again.
@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

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

8 packages built:
  • ebtables
  • ferm
  • libvirt_5_9_0
  • lxd
  • nixops
  • nixopsUnstable
  • nixops_1_6_1
  • shorewall

@Mic92 Mic92 merged commit 90b88c4 into NixOS:master Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@misuzu misuzu misuzu approved these changes

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@dtzWill dtzWill Awaiting requested review from dtzWill

@Mic92 Mic92 Awaiting requested review from Mic92

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@yesbox @misuzu @SuperSandro2000 @Mic92