refactor!: improve reexport structure #3023

dignifiedquire · 2024-12-09T18:02:38Z

Description

Restructures both iroh_base and iroh reexports and type placements, to simplify and clarify the overall structures.

Breaking Changes

iroh-base
- key -> iroh_base::NodeId etc
- hash -> iroh_base::Hash etc
- node_addr -> iroh_base::NodeAddr
- relay_url -> iroh_base::RelayUrl
- relay_map -> use iroh_relay::relay_map
iroh
- hash -> iroh_base
- ticket -> iroh_base::ticket)
- relay -> removed, use iroh_relay directly if needed
- key -> NodeId, etc top level now
- endpoint::Bytes removed use bytes::Bytes
- endpoint::NodeAddr, use iroh::NodeAddr

Notes & open questions

Change checklist

Self-review.
Documentation updates following the style guide, if relevant.
Tests if relevant.
All breaking changes documented.

github-actions · 2024-12-09T18:09:23Z

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 27d8291

dignifiedquire · 2024-12-10T12:04:56Z

I updated iroh-base to look like the above, would love some feedback on this

rklaehn · 2024-12-11T12:05:52Z

iroh-base/src/lib.rs

+    QuicConfig as RelayQuicConfig, RelayMap, RelayNode, DEFAULT_RELAY_QUIC_PORT, DEFAULT_STUN_PORT,
+};
+#[cfg(any(feature = "relay", feature = "key"))]
+#[cfg_attr(iroh_docsrs, doc(cfg(any(feature = "relay", feature = "key"))))]


You should try this out, but I think if you reexport something from a module that has the #[cfg_attr(iroh_docsrs, doc(cfg(any(feature = "relay", feature = "key"))))] thing, all reexports will also have the proper tags in the docs.

So you could simplify this by just having this statement on the (non-pub) module.

Ah, never mind. This is just iroh-base...

Why do we need a const for the size of an ed key in the first place?

Why do we need a const for the size of an ed key in the first place?

we can drop that probably, need to check

Why do we need a const for the size of an ed key in the first place?

it is used in a few places, sorry

Just write 32 or have a const per crate. It is not going to change.

rklaehn · 2024-12-11T12:06:31Z

iroh-base/src/lib.rs

+#[cfg(feature = "relay")]
+#[cfg_attr(iroh_docsrs, doc(cfg(feature = "relay")))]
+pub use self::relay_map::{
+    QuicConfig as RelayQuicConfig, RelayMap, RelayNode, DEFAULT_RELAY_QUIC_PORT, DEFAULT_STUN_PORT,


Not sure I like having these constants at top level. It is probably very rare that somebody needs them, so having them hidden away in some public mod is not that bad.

I didn't find a better way, this is only for iroh-base though, in iroh it is much cleaner now

flub · 2024-12-11T13:44:17Z

iroh-base/src/lib.rs

+#[cfg(feature = "relay")]
+#[cfg_attr(iroh_docsrs, doc(cfg(feature = "relay")))]
+pub use self::relay_map::{
+    QuicConfig as RelayQuicConfig, RelayMap, RelayNode, DEFAULT_RELAY_QUIC_PORT, DEFAULT_STUN_PORT,


I really don't like renames because the compiler will now give users confusing errors. Can you just rename the relay_map::QuicConfig to relay_map::RelayQuicConfig for this?

iroh-base/src/lib.rs

flub · 2024-12-11T13:46:25Z

iroh-dns-server/examples/convert.rs

@@ -1,7 +1,7 @@
 use std::str::FromStr;

 use clap::Parser;
-use iroh::NodeId;
+use iroh::key::NodeId;


Oh, not sure I like this no longer bing on the top level.

flub · 2024-12-11T14:01:27Z

iroh/src/lib.rs

-};
-pub use iroh_relay as relay;
+pub use endpoint::{Endpoint, RelayMode};
+pub use iroh_base::{hash, key, node_addr, relay_map, ticket};


do we need hash?

I'd like to keep NodeId at least at the toplevel.

If we're removing RelayUrl from here then why keep RelayMode? Yes it's used in the builder but if you want anything else then disabled or default you'll need a whole bunch more relay types anyway.

because RelayMode is defined in iroh, not in base

I'd like to keep NodeId at least at the toplevel.

that's fair

do we need hash?

interesting question, we actually don't

hash is gone now

github-actions · 2024-12-11T19:09:28Z

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3023/docs/iroh/

Last updated: 2024-12-12T10:34:00Z

these are all over the place

flub

seems like a decent improvement

flub · 2024-12-12T10:25:15Z

iroh/src/magicsock/node_map/path_state.rs

@@ -6,7 +6,7 @@ use std::{
    time::{Duration, Instant},
 };

-use iroh_base::key::NodeId;
+use iroh_base::NodeId;


I find it a little odd that the imports don't come from crate::NodeId... but I don't think this is a problem. Probably rust-analzyer will keep inserting this anyway.

dignifiedquire added this to the v0.30.0 milestone Dec 9, 2024

rklaehn reviewed Dec 11, 2024

View reviewed changes

flub reviewed Dec 11, 2024

View reviewed changes

dignifiedquire self-assigned this Dec 11, 2024

dignifiedquire force-pushed the refactor-normalize-reexports branch from 504f711 to 4de6e89 Compare December 11, 2024 18:27

dignifiedquire changed the title ~~[WIP] refactor(iroh): improve reexport structure~~ refactor(iroh): improve reexport structure Dec 11, 2024

dignifiedquire marked this pull request as ready for review December 11, 2024 19:03

dignifiedquire changed the title ~~refactor(iroh): improve reexport structure~~ refactor(iroh)!: improve reexport structure Dec 11, 2024

dignifiedquire force-pushed the refactor-normalize-reexports branch from 6ae64b8 to d034234 Compare December 11, 2024 20:42

dignifiedquire added 14 commits December 12, 2024 11:17

refactor(iroh): normalize iroh_base reexports

a0f3992

refactor!: remove RelayUrl reexports

1c04bbe

these are all over the place

cleanup

0d94e12

fixup iroh-relay

8c2096f

update iroh-net-reporpt

c4259cc

fixup imports

66b5a9d

fixup feature

3fad838

fixup docs

142a05a

fixup doc tests

593ee6c

fixup bench

2ff8ef1

fixup docs

2eb7aa9

fixup

fcbf620

typing is hard

f527dd5

fixup

24cddc7

dignifiedquire force-pushed the refactor-normalize-reexports branch from 49faded to 24cddc7 Compare December 12, 2024 10:17

flub approved these changes Dec 12, 2024

View reviewed changes

refactor: move relay_map from iroh-base to iroh-relay

96d0648

dignifiedquire added this pull request to the merge queue Dec 12, 2024

dignifiedquire changed the title ~~refactor(iroh)!: improve reexport structure~~ refactor!: improve reexport structure Dec 12, 2024

Merged via the queue into main with commit d9fb470 Dec 12, 2024
29 of 30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: improve reexport structure #3023

refactor!: improve reexport structure #3023

dignifiedquire commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading

dignifiedquire commented Dec 10, 2024

rklaehn Dec 11, 2024

rklaehn Dec 11, 2024

rklaehn Dec 11, 2024

dignifiedquire Dec 11, 2024

dignifiedquire Dec 11, 2024

rklaehn Dec 12, 2024

rklaehn Dec 11, 2024

dignifiedquire Dec 11, 2024

flub Dec 11, 2024

dignifiedquire Dec 11, 2024

flub Dec 11, 2024

dignifiedquire Dec 11, 2024

flub Dec 11, 2024

dignifiedquire Dec 11, 2024

dignifiedquire Dec 11, 2024

dignifiedquire Dec 11, 2024

dignifiedquire Dec 11, 2024

github-actions bot commented Dec 11, 2024 •

edited

Loading

flub left a comment

flub Dec 12, 2024

refactor!: improve reexport structure #3023

refactor!: improve reexport structure #3023

Conversation

dignifiedquire commented Dec 9, 2024 • edited Loading

Description

Breaking Changes

Notes & open questions

Change checklist

github-actions bot commented Dec 9, 2024 • edited Loading

dignifiedquire commented Dec 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 11, 2024 • edited Loading

flub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dignifiedquire commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 9, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading