Skip to content

[ul] Fix API breakages from past contributions for 0.9.1#740

Merged
Enet4 merged 14 commits intomasterfrom
bug/ul/api-breakage
Mar 21, 2026
Merged

[ul] Fix API breakages from past contributions for 0.9.1#740
Enet4 merged 14 commits intomasterfrom
bug/ul/api-breakage

Conversation

@Enet4
Copy link
Copy Markdown
Owner

@Enet4 Enet4 commented Feb 18, 2026

An oversight had caused some unwanted changes to the dicom_ul public API. This part can then be reconsidered in 0.10.

Summary

  • re-export CloseSocket in association::client module
  • bring back Release trait and release method to ClientAssociation
  • remove type parameter constraints to struct S in association types
    • It is not a good practice to impose constraints at struct level unless absolutely necessary.
  • Add pub fn methods replicating the methods in association traits onto association types, making it so that they continue to work without importing the respective trait.
    • The plan is to remove them in 0.10.0, requiring users to import the traits.
  • Change the visibility of some types and functions to pub(crate), which are implementation details and not meant to be part of the public API.
  • Add a prelude module to dicom_ul.

@Enet4 Enet4 added bug This is a bug A-lib Area: library C-ul Crate: dicom-ul labels Feb 18, 2026
@Enet4 Enet4 force-pushed the bug/ul/api-breakage branch 2 times, most recently from 4cc4e79 to c77df69 Compare February 19, 2026 19:11
@pgimeno4d
Copy link
Copy Markdown
Contributor

Would it be possible to separate the cosmetic changes (cargo fmt) into their own commit, apart from the actual fixes? As is, it's very hard to tell where the actual changes are.

@Enet4
Copy link
Copy Markdown
Owner Author

Enet4 commented Feb 23, 2026

Would it be possible to separate the cosmetic changes (cargo fmt) into their own commit, apart from the actual fixes? As is, it's very hard to tell where the actual changes are.

You're right, I will work on that when I am able.

@Enet4 Enet4 force-pushed the bug/ul/api-breakage branch from c77df69 to 4dd9a5c Compare February 26, 2026 10:21
@Enet4
Copy link
Copy Markdown
Owner Author

Enet4 commented Feb 26, 2026

OK, the PR should be a bit more readable now.

@Enet4 Enet4 marked this pull request as ready for review February 26, 2026 11:07
@Enet4 Enet4 force-pushed the bug/ul/api-breakage branch from 4dd9a5c to bf4b071 Compare February 26, 2026 11:47
@Enet4 Enet4 added this to the DICOM-rs 0.9.1 milestone Mar 3, 2026
@Enet4 Enet4 force-pushed the bug/ul/api-breakage branch from bf4b071 to d952d79 Compare March 5, 2026 10:26
@Enet4
Copy link
Copy Markdown
Owner Author

Enet4 commented Mar 5, 2026

Well, there are more breaking changes in the code than what was originally predicted. While we can bring back most of the capabilities back, the fact that the return type of establish_async had been changed from ServerAssociation to AsyncServerAssociation in #679 had been overlooked. This is something that cargo-semver-checks does not detect at the moment (obi1kenobi/cargo-semver-checks#149).

I'm starting to think that a full semver compatibility recovery is not fully possible without yanking #679, which would also be too much effort.

So we either go straight for 0.10.0 or proceed with merging this and release 0.9.1 anyway, admitting the differences as minor to the library user. Still, I will need to document them carefully and analyze the potential impact.

@Enet4 Enet4 force-pushed the bug/ul/api-breakage branch from f9bda55 to 777a9a7 Compare March 5, 2026 16:26
@pgimeno4d
Copy link
Copy Markdown
Contributor

In case it helps, I favor 0.10.0, as there were important structural changes that needed to be addressed ASAP, and consider 0.9 as a release that serves as a transition into one that will be more stable in the long term.

@Enet4
Copy link
Copy Markdown
Owner Author

Enet4 commented Mar 6, 2026

I have updated this PR with more constructs for compatibility. Inspecting the differences in documentation, the only major difference becomes the return type of (Client|Server)AssociationOptions::establish_async, from (Client|Server)Association<_> to (Client|Server)AsyncAssociation<_>. Structurally speaking they are equivalent, so any pre-existing method calls (send, receive, etc.) will continue to work. Only code attempting to match the type of the established association value will fail.

@Enet4 Enet4 force-pushed the bug/ul/api-breakage branch 4 times, most recently from 6d95fa6 to 8f25dae Compare March 7, 2026 16:53
Enet4 added 8 commits March 12, 2026 12:31
- Re-export CloseSocket in `association::client` module
- Bring back `Release` trait as deprecated
- Extend documentation of `SyncAssociation::release`
- empty `non_blocking` modules
- remove constraint `S: Read + Write + CloseSocket` from `ServerAssociation` struct
- method `ServerAssociation::client_ae_title`
…ncode_pdu`

They are implementation details.
…tion types

- Adds more compatibility to code using 0.9.0,
so that it continues to work without importing the respective trait.
  Should be removed in 0.10.0.
- Update test code accordingly
  (no longer needs an import)
Enet4 added 5 commits March 12, 2026 12:31
- easily import all association traits
- add pub functions to `ClientAssociation` and `AsyncClientAssociation`:
  `user_variables`, `requestor_max_pdu_length`, `acceptor_max_pdu_length`, `presentation_contexts`
@Enet4 Enet4 force-pushed the bug/ul/api-breakage branch from 069893d to fede7d7 Compare March 12, 2026 12:31
- move `InvalidPduFieldLength` and `ReadUserVariable` to the end
@Enet4 Enet4 merged commit 38ff5b3 into master Mar 21, 2026
5 checks passed
@Enet4 Enet4 deleted the bug/ul/api-breakage branch March 21, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-lib Area: library bug This is a bug C-ul Crate: dicom-ul

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants