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

Non-additive features yield BollardDate incompatibility #462

Open
jwest23 opened this issue Sep 9, 2024 · 2 comments
Open

Non-additive features yield BollardDate incompatibility #462

jwest23 opened this issue Sep 9, 2024 · 2 comments
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@jwest23
Copy link

jwest23 commented Sep 9, 2024

I am using bollard twice; once directly, and once indirectly.

Indirectly, I am using bollard via the dockertest crate. It depends on bollard 0.17.1 with the default feature set. Without any further interference, bollard-stubs ends up with the BollardDate type alias for String. The dockertest crate understandably works with bollard as if BollardDate is a String.

Directly, I am using bollard to run containers for other purposes and have enabled the buildkit feature. This enables the chrono feature. With chrono enabled, bollard-stubs ends up with the BollardDate type alias for chrono::DateTime<chrono::Utc>> which seems right. That substitution is done here.

The result of this is that I cannot have both dockertest and bollard with buildkit enabled in the same crate. When I try to compile, I end up with errors where the dockertest crate is trying, for example, to call .as_str() on a timestamp from bollard_stubs::models::Network.created. The dockertest crate is expecting that to be an Option<String> and this would compile fine until the chrono feature is introduced. When chrono is enabled, the type changes to Option<chrono::DateTime<chrono::Utc>> and chrono::DateTime<chrono::Utc>> does not support the .as_str() method.

Is there any way to resolve this quickly that I've missed? I could try to get rid of my dependence on the buildkit feature. Failing that, the only solutions I see are to replace every BollardDate instance with String or to make BollardDate a type instead of a type alias so that it can have a well-defined interface that does not change based on the enabled features. The latter two are no small amount of work, but I'm hoping to avoid the former. If there's another path through, I'd appreciate you pointing me in the right direction.

Thank you for the fantastic crate and for giving this your attention!

@fussybeaver
Copy link
Owner

That's unfortunate... I guess the buildkit code path needs to use the appropriate flags to toggle chrono - which doesn't look like a significant amount of work (I can take a look after PTO in a few weeks).

For the immediate fix, I can only think that you compile and test without buildkit, and re-enable it when building the release.

@fussybeaver fussybeaver added the bug Something isn't working label Sep 10, 2024
@fussybeaver
Copy link
Owner

I was taking another look at this in https://github.com/fussybeaver/bollard/compare/ND%2Fbuildkit-datetime?expand=1 , but one problem is that the registry token authentication used in some buildkit operations returns an rfc3399 date that isn't easily converted into a timestamp without one of the chrono or time libraries, which is needed for the buildkit protobuf serialisation.

So... I'll mark this one as "won't fix" for the moment.

@fussybeaver fussybeaver added the wontfix This will not be worked on label Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants