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

recursive dependency with github.com/coreos/go-systemd #73

Closed
mwhudson opened this issue Jun 21, 2016 · 12 comments
Closed

recursive dependency with github.com/coreos/go-systemd #73

mwhudson opened this issue Jun 21, 2016 · 12 comments

Comments

@mwhudson
Copy link

[re-filing of https://github.com/coreos/go-systemd/issues/183]

Hi,

Currently github.com/coreos/pkg/capnslog/journald_formatter imports github.com/coreos/go-systemd/journal, but github.com/coreos/go-systemd/journal and github.com/coreos/go-systemd/sdjournal/journal import github.com/coreos/pkg/dlopen. This is a pain for distro developers: we want to package both github.com/coreos/go-systemd and github.com/coreos/pkg but cycles in the dependency graph are a pain. In this case it looks as if .../pkg/dlopen could move to ../go-systemd but I've no idea if that would really be appropriate.

Splitting things out into separate repos would also always be a fix for this.

Cheers,
mwh

@mwhudson
Copy link
Author

Replying to @lucab 's questions on the other bug...

Hi and thanks for the report! Sorry for the dependency loop, I think we can easily manage to break it.

Yay, thanks.

However I'm not convinced about moving dlopen to go-systemd, as we (do or may) use dlopen in other places too.

Fair enough.

Alternative suggestions:

Break dlopen in its own separate go-dlopen project
Break capnlog in its own separate go-capnlog project. There is some discussion ongoing to completely drop capnlog #57
@mwhudson opinions on those? In both case I think this discussion should be moved to coreos/pkg tracker, can you please move this dicussion there (or I'll do later)?

Either works perfectly well for me.

Side question: different distros have different policies, are you looking into a specific one?

I'm coming from the Debian/Ubuntu world.

@mwhudson
Copy link
Author

Ping? Is there anything I can do to help?

@lucab
Copy link
Contributor

lucab commented Jun 23, 2016

Pinging @iaguis and @barakmich, who have definitely better knowledge than me on dlopen and capsnlog.

My suggestion would still be to split dlopen out of here. @tmrts was however suggesting to take this occasion to start deprecating (via splitting) capsnlog.

@jonboulle
Copy link
Contributor

I think I'd rather split out capnslog since dlopen is a small amount of utility code and capsnlog is a full log framework (and, as discussed, potentially to be deprecated)

@mwhudson
Copy link
Author

Unfortunately splitting out capnslog doesn't directly solve the problem because pkg/netutil imports it, so the cycle would remain (just bigger). Can netutil move somewere else too? Or stop importing it? Or something else?

This cycle is starting to interfere with my work so if there's any spade work I can do to help I'm certainly willing to do that.

@squeed
Copy link
Collaborator

squeed commented Jun 30, 2016

pkg only has 3 non-stdlib dependencies:

  • github.com/coreos/go-systemd/journal
  • golang.org/x/crypto/ssh/terminal
  • gopkg.in/yaml.v1

If we remove the requirement on go-systemd/journal, then pkg will have an extremely clean import story. The only functionality we're using from journal is a 200-line file that generates a systemd-journal UDP packet. Perhaps that can just be moved out of go-systemd and into pkg?

@lucab
Copy link
Contributor

lucab commented Jun 30, 2016

It also uses journal.Enabled() but yes, duplicating that single file would buy us some time. #38 however goes in the opposite direction.

@mwhudson
Copy link
Author

Ping, any progress/more thinking here? Again, anything I can do to help?

@jonboulle
Copy link
Contributor

I merged #75 - I think we should push on with splitting out capnslog into its own repo

mwhudson added a commit to mwhudson/pkg that referenced this issue Aug 1, 2016
@mwhudson
Copy link
Author

mwhudson commented Aug 1, 2016

Well that seems easy enough, I think https://github.com/mwhudson/capnslog needs to become https://github.com/coreos/capnslog and then something like https://github.com/mwhudson/pkg/tree/no-capnslog and that's all?

jonboulle added a commit to jonboulle/pkg that referenced this issue Aug 4, 2016
This eliminates a circular dependency between coreos/pkg and
coreos/go-systemd.

See coreos#73 for more details.
@onlyjob
Copy link

onlyjob commented Dec 18, 2019

As of v22.0.0, go-systemd no longer depends on github.com/coreos/pkg:
coreos/go-systemd@3344f03f

@lucab
Copy link
Contributor

lucab commented Dec 18, 2019

Oh, I totally forgot about this ticket. We embedded the logic there in order to untangle the mess around versioning and Go modules.
@onlyjob thanks for pinging back here, I'm closing this now.

@lucab lucab closed this as completed Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants