storage icon indicating copy to clipboard operation
storage copied to clipboard

RFC: Remove drivers/windows

Open mtrmac opened this issue 8 months ago • 5 comments

Compare https://github.com/containers/storage/issues/2009 : The code could never build its unit tests since September 2017, and nobody seems to have complained.

Also, see how this impacts go.mod: many of the largest (google.golang.org/grpc), obsolete (github.com/gogo/protobuf) or outright frozen and unmaintained (go.opencensus.io), or just aesthetically unappealing (github.com/containerd/…) dependencies go away.

[And then we could discuss the unmaintained github.com/json-iterator/go, a codebase full of reflect and unsafe. That’s also fun.]

To be fair, containers/ocicrypt drags GRPC into Podman anyway, and several dependencies add various implementations of protobuf.

I’m not going to strongly advocate merging this, but I did want to show that, look, this is clearly not in a good shape, and the cost is not all that small, just as one more data point for the conversation (so, marking as draft, ~indefinitely).

Cc: @giuseppe @nalind @mheon

But then again, would this impact plans for https://github.com/containers/buildah/issues/4010 ? Do we instead need to start seriously caring? Cc: @sebsoto

mtrmac avatar Mar 14 '25 04:03 mtrmac

Cc: @giuseppe @nalind @mheon

But then again, would this impact plans for containers/buildah#4010 ? Do we instead need to start seriously caring? Cc: @sebsoto

I am in favor of simplifying code that is out of maintenance (as it appears to be in this case). (+1. -89,831) is a significant improvement, even if it doesn't affect the binary size

giuseppe avatar Mar 14 '25 09:03 giuseppe

LGTM on my end

mheon avatar Mar 14 '25 10:03 mheon

But then again, would this impact plans for https://github.com/containers/buildah/issues/4010 ? Do we instead need to start seriously caring?

Properly best to wait on some come of outcome on that discussion to see whenever that will be a priority or not. Removing this now just to add it back in a few months doesn't seem particular helpful either.

The diff looks very appealing though, so I would love for this to just be gone...

Luap99 avatar Mar 14 '25 11:03 Luap99

Four months later…

I want to acknowledge that some discussions about native Windows support are ongoing, but so far I don’t read them to suggest that it is very likely to happen. (Also, none of the proponents of Windows support have spoken to support this in this repo, AFAIK) Please correct me if anything of the above is wrong.

So, shall we pull the plug and delete the driver now? We can always revert later. Marking as “Ready for review”.

mtrmac avatar Jul 17 '25 17:07 mtrmac

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, Luap99, mtrmac

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • ~~OWNERS~~ [giuseppe,mtrmac]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci[bot] avatar Jul 18 '25 02:07 openshift-ci[bot]