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

[wireguard][1.0.20210219] - Updated package (and include wireguard-tools) #285

Merged
merged 13 commits into from
Mar 12, 2021

Conversation

jonahweissman
Copy link

As promised in #270 (comment), I've created a recipe to package wireguard-tools. In addition, to getting rid of files that were only relevant on OpenWRT, this recipe also includes wg-quick(8), which makes it easy to set up a wireguard tunnel with systemd.

I also updated the version of wireguard to 1.0.20210124 in this same PR, since they're so tightly coupled; I hope that's okay.

There are a few things I'd like to discuss:

  1. The wireguard-tools source archive includes a symbolic link to wg, which is not present until the package is built. Automatically extracting the source caused a bunch of errors around that symlink, so I did it manually in the prepare section and ended up still having to create a dummy file. This seems like a pretty rare situation, so it's probably fine to have an ad hoc solution like this, but it might make more sense to make the tooling ignore broken symlinks in source archives.
  2. I named the package wg-tools so that it wouldn't conflict with wireguard-tools in Entware. I don't know how conflicts are resolved, but, ideally, this package would also be named wireguard-tools.
  3. I made wireguard depend on wg-tools, but not the reverse. This way, users removing wireguard will automatically remove wg-tools without having to use any force flags. Is there a good way to discourage anyone from installing wg-tools directly?
  4. I put wireguard-tools in a separate directory, but this seems a little messy since they are so closely related.

@raisjn
Copy link
Contributor

raisjn commented Feb 20, 2021

i don't have anything to add here, sorry. maybe @matteodelabre, @LinusCDE or @Eeems?

@danshick
Copy link
Member

danshick commented Feb 20, 2021

1.) We should open a separate toltec issue for the symlink handling issue.

2.) You may want to call your package something like wireguard-tools-remarkable and have it "provide" wireguard-tools.

3.) Not without having the bidirectional dependency. There isn't a usecase for wireguard-tools without wireguard, so that would be reasonable to do.

4.) You can put both tools on the same package recipe if you like. Look at rmkit for an example.

Lastly, any reason you're not installing the manuals?

@jonahweissman
Copy link
Author

1.) We should open a separate toltec issue for the symlink handling issue.

Done: #291.

2.) You may want to call your package something like wireguard-tools-remarkable and have it "provide" wireguard-tools.

That makes sense, but I don't think Toltec currently supports the provides keyword. Is there another way to do it?

3.) Not without having the bidirectional dependency. There isn't a usecase for wireguard-tools without wireguard, so that would be reasonable to do.

Okay, a bidirectional dependency definitely makes sense. However, with the bidirectional dependency, this happens:

reMarkable: ~/ opkg remove wireguard wg-tools
No packages removed.
Collected errors:
 * print_dependents_warning: Package wireguard is depended upon by packages:
 * print_dependents_warning:    wg-tools
 * print_dependents_warning: These might cease to work if package wireguard is removed.

 * print_dependents_warning: Force removal of this package with --force-depends.
 * print_dependents_warning: Force removal of this package and its dependents
 * print_dependents_warning: with --force-removal-of-dependent-packages.
 * print_dependents_warning: Package wg-tools is depended upon by packages:
 * print_dependents_warning:    wireguard
 * print_dependents_warning: These might cease to work if package wg-tools is removed.

 * print_dependents_warning: Force removal of this package with --force-depends.
 * print_dependents_warning: Force removal of this package and its dependents
 * print_dependents_warning: with --force-removal-of-dependent-packages.

It's obviously pretty easy to resolve, but it's not a friendly experience for users.

4.) You can put both tools on the same package recipe if you like. Look at rmkit for an example.

I thought about this, but if I understand correctly, even in split packages, there is only one build function. Since wireguard-tools and wireguard-linux-compat come in separate source archives, I would have to do a lot of shuffling around in the build phase, which might negate the tidiness of having them both in the same recipe.

Lastly, any reason you're not installing the manuals?

No, not really. I think I just made some assumptions based on #235. I've added them back.

@danshick
Copy link
Member

danshick commented Feb 22, 2021

That makes sense, but I don't think Toltec currently supports the provides keyword. Is there another way to do it?

You're right. We had one PR (that looks stale) to add a replaces field that would have implemented this type of behavior. We had also talked about making the rm2fb package "provide" a display metapackage. Not sure where that discussion took place. I wrongly assumed we had tooling support for that.

Edit: The plan was to make display a metapackage that strictly depended on rm2fb for rM2 and nothing for rM1, so the relationship was defined the other way around.

This may also deserve its own issue.

It's obviously pretty easy to resolve, but it's not a friendly experience for users.

Ugh, yeah, that's not great. This should resolve more cleanly. Maybe you could just flip the dependency around? Make wg-tools depend on wireguard. That makes wireguard independently installable without wg-tools though, which is a little weird.

This is sort of what Arch does, by having no dependencies in wireguard-dkms, but having optdepends in wireguard-tools on wireguard-dkms. This is a little different though as wireguard is in the kernel tree by default for Arch.

Or maybe we need a "wireguard" metapackage that depends on two much more obscurely named packages wireguard-module-remarkable and wireguard-tools-remarkable. This also fixes your overlapping naming problem in point 2.

One downside to this is that there are alternatives to wireguard-linux-compat out there like boringtun, but that's something to think about if/when it gets packaged I suppose.

I thought about this, but if I understand correctly, even in split packages, there is only one build function. Since wireguard-tools and wireguard-linux-compat come in separate source archives, I would have to do a lot of shuffling around in the build phase, which might negate the tidiness of having them both in the same recipe.

If the build script is cleaner separately, that's fine.

No, not really. I think I just made some assumptions based on #235. I've added them back.

Maybe a split package actually does make sense here, but for man pages, not the module/tooling.

Given other packages don't currently install manuals, this is fine the way it is for now. Easy enough to adjust down the road.

@jonahweissman
Copy link
Author

I just realized that almost every single one of these problems can be resolved by putting the wireguard-tools files in the wireguard package. Are there any downsides you can think of?

@danshick
Copy link
Member

Only the possible future inclusion of a boringtun (or similar) package, but we can sort that out if/when someone packages it.

@matteodelabre
Copy link
Member

matteodelabre commented Mar 1, 2021

Some other ideas regarding (2.):

  • As long as your version is greater than the one in Entware, opkg will always favor your package. You could set your package’s epoch to a greater number than the one in Entware (i.e., pkgver=1:1.0.20200827-1). Epochs rarely get bumped, so this would work most of the time.
  • Opkg has an architecture priority feature that does not exist in Debian: it will always install the package that targets the architecture with the highest priority (as configured in /opt/etc/opkg.conf). By default, Entware packages target the armv7-3.2 architecture with priority 160. We could set the package architecture to a special value that has a priority higher than 160 (see also our discussion around adding an architecture field to recipes, and the initial implementation on the tooling/split-architectures branch).

@jonahweissman
Copy link
Author

Only the possible future inclusion of a boringtun (or similar) package, but we can sort that out if/when someone packages it.

Sounds good. I've just pushed a version of the wireguard package that includes the wireguard-tools files.

  • As long as your version is greater than the one in Entware, opkg will always favor your package.
  • Opkg has an architecture priority feature that does not exist in Debian

This is good to know. For this particular case, I think it'll be simplest just to combine the packages.

@jonahweissman jonahweissman changed the title [wireguard-tools][1.0.20200827] - New package [wireguard][1.0.20210219] - Updated package (and include wireguard-tools) Mar 1, 2021
@matteodelabre matteodelabre added the packages Add or improve packages of the repository label Mar 4, 2021
@danshick
Copy link
Member

danshick commented Mar 7, 2021

I know this one has been sitting out here a while. I can and will test it, but it may be a week or so still before I can make time. Apologies for that.

Copy link
Member

@danshick danshick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package installs and works great on rM2. @Eeems and @matteodelabre agree the simplified approach of calling both the kernel module and tools wireguard works for now. LGTM

@danshick danshick merged commit c0339cf into toltec-dev:testing Mar 12, 2021
raisjn pushed a commit to rmkit-dev/toltec that referenced this pull request Mar 23, 2021
new packages:
    [ddvk-hacks] Add ddvk-hacks (toltec-dev#247)

updated packages:
    [wireguard][1.0.20210219] - Updated package (and include wireguard-tools) (toltec-dev#285)
    [rm2fb] update rm2fb to work with xochitl 2.6 (v1.0.1) (toltec-dev#301)
    [recrossable] Update recrossable (toltec-dev#312)
    [wikipedia] Initial wikipedia package.
    [appmarkable] Update appmarkable to 0.0.0-9 and rmservewacominput to 0.3.0-1 (toltec-dev#308) with rm2 support
    [rmkit] patch genie to fix crash in testing (toltec-dev#304)
    [oxide] Update Oxide to v2.1.2 (toltec-dev#241)
    [rm2fb] update rm2fb with wait ioctl and no-op on rM1 (toltec-dev#298)
    [rmkit] add bufshot app, add lamp, add iago, add changelog (toltec-dev#276)
    [rmkit] update rmkit to latest (2021-02-17) (toltec-dev#286)
    [zshelf][0.3.1] - Updated Package (toltec-dev#287)

tooling:
    Pin the Ubuntu version used in workflows to 20.04 (toltec-dev#316)
    Provide better version number error messages (toltec-dev#314)
    util.auto_extract: Extract broken symlinks and missing directories (toltec-dev#302)
    change web background color to #fcfaf8 (toltec-dev#280)
    Implement build-time package dependencies (toltec-dev#274)
    Rewrite repo-build-web in Python (toltec-dev#266)
    Print last 50 lines of output on build error (toltec-dev#263)
    Hardcode REMOTE_HTTP secret in PR workflows (toltec-dev#262)
    Rewrite repo-build and package-build in Python (toltec-dev#218)
    Make bootstrap execution conditional on hash verification (toltec-dev#257)
    Add Toltec web home page (toltec-dev#193)
danshick pushed a commit to danshick/toltec that referenced this pull request May 5, 2021
…ols) (toltec-dev#285)

Add wireguard:

* Update wireguard and clean up recipe
* Create wg-tools package
* Update wireguard to 1.0.20210219
* Include man pages
* Add wireguard-tools files to wireguard
* Remove separate wireguard-tools package
danshick added a commit to danshick/toltec that referenced this pull request May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Add or improve packages of the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants