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

Add dag package #2141

Merged
merged 3 commits into from Jun 5, 2023
Merged

Add dag package #2141

merged 3 commits into from Jun 5, 2023

Conversation

bufdev
Copy link
Member

@bufdev bufdev commented May 31, 2023

Competing idea to #2138.

This started as a vendoring of https://github.com/stevenle/topsort with proper license attribution, plus some cleanups to more closely match our code standards, but evolved into more functionality and more cleanups. We can edit this as we want for our needs. It's a super-simple package, and this is a universal concept.

@unmultimedio any thoughts about starting with this as a base?

@bufdev bufdev requested a review from unmultimedio May 31, 2023 16:09
@bufdev bufdev changed the title Add toposort package Add dag package Jun 3, 2023
@bufdev
Copy link
Member Author

bufdev commented Jun 5, 2023

I've spent some more time on this in support of the direct-deps branch, and I think this is the better way to go here - it's more full-fledged now, and is mostly up to our standards.

@unmultimedio
Copy link
Member

I've spent some more time on this in support of the direct-deps branch, and I think this is the better way to go here - it's more full-fledged now, and is mostly up to our standards.

Sure! Let's go with this one if you see it works as we need it. What I'm missing from this package (probably I'm not thinking it or checking it through), are these two:

  1. how to sort a complete graph, w/out giving it any starting point, something like: sort the whole tree, return the sorted modules on it. Useful for workspace push or sync managed modules. Probably it's enough to add everything under the same "root", and then request to have the "root" node sorted.
  2. are non-dependent nodes in the tree lexicographically sorted or any other way to they're deterministic?

@bufdev
Copy link
Member Author

bufdev commented Jun 5, 2023

  1. how to sort a complete graph, w/out giving it any starting point, something like: sort the whole tree, return the sorted modules on it. Useful for workspace push or sync managed modules. Probably it's enough to add everything under the same "root", and then request to have the "root" node sorted.

Isn't that just:

graph := NewGraph()
graph.Add("root") // whatever your concept of the starting point is
graph.TopoSort("root")

?

  1. are non-dependent nodes in the tree lexicographically sorted or any other way to they're deterministic?

I don't understand the question, can you clarify?

@unmultimedio
Copy link
Member

unmultimedio commented Jun 5, 2023

  1. Yeah, that should be enough, thx
  2. If a->b, a->c is graph.Sort("a") always b, c, a? or is the order non-deterministic? Because c, b, a is also a valid topological sort.

@bufdev
Copy link
Member Author

bufdev commented Jun 5, 2023

Order is always deterministic, it uses the sort order of the keys, so it will be b, c, a.

@bufdev bufdev merged commit 8064423 into main Jun 5, 2023
9 checks passed
@bufdev bufdev deleted the toposort branch June 5, 2023 17:27
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

Successfully merging this pull request may close these issues.

None yet

2 participants