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

Move CachePadded to separate sub-crate? #1066

Open
mgeier opened this issue Jan 2, 2024 · 5 comments · May be fixed by #1075
Open

Move CachePadded to separate sub-crate? #1066

mgeier opened this issue Jan 2, 2024 · 5 comments · May be fixed by #1075
Labels

Comments

@mgeier
Copy link

mgeier commented Jan 2, 2024

I have been using https://github.com/smol-rs/cache-padded/ as a dependency until it has been deprecated (smol-rs/cache-padded#11) and then, as suggested, I switched to crossbeam_utils::CachePadded.

I didn't really like that this forced an unneeded dependency (cfg-if, mentioned at smol-rs/cache-padded#10 (comment)) onto my crate.

I kinda accepted this, but recently, I found another inconvenience: it also forces me to bump the MSRV of my crate due to #1037, which is AFAICT unrelated to CachePadded.

Would it be possible to move CachePadded to a separate sub-crate to avoid those problems?

Alternatively, both problems might be avoidable by moving parts of crossbeam_utils to separate "features" (which I could deactivate for my crate), but I don't know if that's feasible?

@taiki-e
Copy link
Member

taiki-e commented Jan 3, 2024

I didn't really like that this forced an unneeded dependency (cfg-if, mentioned at smol-rs/cache-padded#10 (comment)) onto my crate.

Well, as for cfg-if I don't really like it and will eventually remove it. Work on revisiting dependencies is in progress, and a few dependencies were recently removed from some sub-crates (#1058, #1045).

I kinda accepted this, but recently, I found another inconvenience: it also forces me to bump the MSRV of my crate due to #1037, which is AFAICT unrelated to CachePadded.

Do you have any concrete requirements? I think we can consider relaxing MSRV if it is really needed. For example, #1056 has relaxed the MSRV a bit that #1037 bumped.

Would it be possible to move CachePadded to a separate sub-crate to avoid those problems?

Alternatively, both problems might be avoidable by moving parts of crossbeam_utils to separate "features" (which I could deactivate for my crate), but I don't know if that's feasible?

One of the reasons cache-padded was deprecated was the maintenance burden, such as reflecting updates in crossbeam-utils. If we move the cache-padded to crossbeam repo and making the implementation a symbolic link to the crossbeam-utils code, it would be possible to reduce the maintenance burden. That said, I'm not sure it is worth having a separate crate for it, as said in smol-rs/cache-padded#10 (comment). (I'm not interested in adding new dependencies to crossbeam-utils.)

@mgeier
Copy link
Author

mgeier commented Jan 7, 2024

Well, as for cfg-if I don't really like it and will eventually remove it.

That's great news!

Do you have any concrete requirements?

No. I just dislike bumping the MSRV of my crate without any real reason. Every bump is a potential obstacle for users of my crate and of dependent crates.
And even if the current MSRV is not a problem, there might be future bumps in crossbeam-utils that would not be necessary for CachePadded.

I'm not against bumping the MSRV in general, I just would like to avoid doing it gratuitously.

If we move the cache-padded to crossbeam repo and making the implementation a symbolic link to the crossbeam-utils code, it would be possible to reduce the maintenance burden.

With an additional sub-crate there would of course be an added maintenance cost when updating crate versions, but I would hope that it is small enough to still be feasible.

I'm not interested in adding new dependencies to crossbeam-utils.

Yeah, I understand, but this would only be an "internal" dependency to a sub-crate.

To make my suggestion a bit more concrete, I've crated a draft PR: #1069.

There are a few details which would have to be ironed out, but it can hopefully serve as a basis for discussion.

What do you think about it?

@taiki-e
Copy link
Member

taiki-e commented Jan 8, 2024

Well, as for cfg-if I don't really like it and will eventually remove it.

That's great news!

Filed #1072 for this.

Do you have any concrete requirements?

No. I just dislike bumping the MSRV of my crate without any real reason. Every bump is a potential obstacle for users of my crate and of dependent crates.

The MSRV was not bumped for no reason. It was just bumped for APIs you are not using (#1033).

I'm not interested in adding new dependencies to crossbeam-utils.

Yeah, I understand, but this would only be an "internal" dependency to a sub-crate.

To make my suggestion a bit more concrete, I've crated a draft PR: #1069.

I said "new dependencies", not "new public dependencies" or "new external dependencies". "new 'internal' dependency" is also a "new dependency". Your PR seems to be doing exactly what I thought I didn't want to do when I wrote that comment.

(Also, your comments make me feel that you are saying, "I don't like the fact that my crate's dependency has a dependency, but I would like you to add a dependency to your crate".)

@taiki-e
Copy link
Member

taiki-e commented Jan 8, 2024

Well, as for cfg-if I don't really like it and will eventually remove it.

That's great news!

Filed #1072 for this.

This has been released in crossbeam-utils v0.8.19.

@mgeier
Copy link
Author

mgeier commented Jan 8, 2024

Thanks for removing cfg-if, that's great!

The MSRV was not bumped for no reason. It was just bumped for APIs you are not using (#1033).

Sure, you didn't just bump it for the fun of it. But when I bump the MSRV of my crate, there will be no real reason.

I said "new dependencies", not "new public dependencies" or "new external dependencies". "new 'internal' dependency" is also a "new dependency". Your PR seems to be doing exactly what I thought I didn't want to do when I wrote that comment.

Yeah, I assumed that "internal" dependencies are fine, but I guess they also cause some overhead when downloading crates.

I have created an alternative approach (based on your comment about symbolic links above): #1075.
Would that be more reasonable?

(Also, your comments make me feel that you are saying, "I don't like the fact that my crate's dependency has a dependency, but I would like you to add a dependency to your crate".)

Yeah, I didn't intend it to sound like this. I assumed (wrongly) that "internal" dependencies don't count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants