shared: Move idmap from internal

[hallyn]

Move internal/idmap/* back to shared/idmap/. It was in use by projects like github.com/project-stacker/stacker.

Create an idmapset_other.go so that non-linux systems will just get an error message if they try to use it.

The #included .h files need to move somewhere, but I'm not sure where is best. As is, incus will compile, but stacker for instance will not, since it won't be able to find internal/cgo/*.h.

[hallyn]

tgraber marked this pull request as draft

Oh - yeah - clearly :)

[hallyn]

So if there are any suggestions of where to put the .h files, those would be appreciated - otherwise I'll just try a few things.

[stgraber]

Ideally, I'd make shared/idmap only include the pure-Go stuff.
Then move the rest to internal/linux/idmap so that's where the .h and the cgo bits would go.

The stuff in internal/linux/idmap can then either be just loose exported functions or if it makes sense to tie them to an idmapset or an idmap, then we could have an extended version of those types be defined in internal/linux/idmap, inheriting from those in shared/idmap.

[hallyn]

Yeah - it's simpler than I thought. I was thinking when you import github.com/lxc/incus/shared/idmap, it would actually fetch only that subdir. But incus.git/internal/ is available to it, so it's fine. I'll update later today. Thanks.

[stgraber]

Yeah, Go doesn't really do per-directory imports unless they have their own go.mod.

That said we effectively have a policy to not have packages in shared import anything from internal as the point of shared is that it's public API and stuff folks can also copy/paste in their projects. An import from internal would prevent the copy/paste aspect of it.

The other way is perfectly fine though, packages in internal can absolutely import from shared and all the stuff that's outside of shared (like our commands) are absolutely free to import from internal.

So having things layed out as described above where shared/idmap is just the Go bits that most folks need, then internal/linux/idmap has the weirder Linux bits that folks generally don't need, should work fine.

Obviously this all assumes that in your own case you don't need those Linux specific bits.
From what you told me earlier, you were mostly after the Idmap and IdmapSet structs, which should be fine in this case.

[hallyn]

@stgraber no, that doesn't suffice. Stacker (and project-machine, and atomix) need the uid shifting code as well, and therefore need the cgo bits e.g. for setting v3 filecaps. So if the policy is to not import from internal into shared, then it becomes harder again :)

[stgraber]

@stgraber no, that doesn't suffice. Stacker (and project-machine, and atomix) need the uid shifting code as well, and therefore need the cgo bits e.g. for setting v3 filecaps. So if the policy is to not import from internal into shared, then it becomes harder again :)

Okay, so yeah, if you need the "fun" bits of the package too, then we'll need to move the whole thing. I'll take a look at it later today to see how that may look like.

[hallyn]

Thanks -fwiw the pr as is compiles and allows stacker to compile.

[stgraber]

ERROR: found short form imports: shared/idmap/idmapset_other.go:5:import "fmt"

[hallyn]

ERROR: found short form imports: shared/idmap/idmapset_other.go:5:import "fmt"

Oh yeah - saw that this morning and cursed stupid linters.

[hallyn]

Are zfs tests known to be flaky, or could this be a real failure?

[stgraber]

Some of those tests can be flaky, it's all good now.

[hallyn]

I'd considered moving internal/cgo to shared/cgo, but that made it sound like something others would want to pull so I was trying to think of another way to do that. Other than shared/idmap/cgo which is wrong since not only idmap uses it, I hadn't come up with anything. maybe shared/cgo is just fine.

[stgraber]

I'd considered moving internal/cgo to shared/cgo, but that made it sound like something others would want to pull so I was trying to think of another way to do that. Other than shared/idmap/cgo which is wrong since not only idmap uses it, I hadn't come up with anything. maybe shared/cgo is just fine.

Yeah, it's not great but the fact that it does absolutely nothing when you import it (other than its side effects), makes it pretty easy to remove down the line at least without people getting too mad at us :)