-
Notifications
You must be signed in to change notification settings - Fork 27
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
No direct migration path for whoami::hostname #117
Comments
Thanks for the issue, and PR! Yeah, that function isn't functionally identical (on purpose). I'm not sure what the purpose of adding that function to whoami would be. Converting to lowercase should probably only be done for certain display purposes (and even there I'm not sure), and hostname casing should otherwise be kept. If possible, can you tell me what you're trying to do with a lowercase-normalized hostname, and why the casing shouldn't be preserved? Is a bug caused by not making it lowercase? The reasoning for this change is discussed here: #82 |
I'm doing a mass change of other people's code, I'm not exactly sure what the precise requirements are in each case. It's quite possible that it makes no difference at all, but I'm not in a position to be able to evaluate every callsite. We have had problems in the past from strange failures relating the case-mismatches in hostnames (eg case-sensitive string cmp & mismatch in case between hostname() and a hostname in a config file), so I'd be hesitant about just assuming the case issue can be ignored. BTW This is to resolve the possible security issue arising from #91. So I really want a very direct migration path without having to worry about any semantic differences. |
Without a requirement, I'm hesitant to add this as a new function to whoami. It seems like an okay function for consumers of the library to have to write, if they really need it.
To me, this sounds like switching to the new API would have fixed some of those old issues, so I'm questioning the value of the additional API in whoami besides as an intermediate step in addressing tech-debt.
While that function is deprecated, it's not going away any time soon (whoami 2.0 will likely be released in mid-2025, and 1.0 will still be maintained). I think it would be okay to |
I've fallen back to scattering a pile of |
Is your feature request related to a problem? Please describe.
whoami::hostname
is deprecated. The suggested replacement iswhoami::fallible::hostname
, but this is not functionally identical. In principle the case of hostname shouldn't matter but in practice it means that we can't just directly replace calls towhoami::hostname()
towhoami::fallible::hostname().unwrap_or_else(|| "localhost".to_string())
(or just expect/unwrap) without worrying about introducing a regression.Describe the solution you'd like
Maybe
whoami::fallible::hostname_lower()
which is functionally identical to the infallible version (aside from returning a Result).Describe alternatives you've considered
Open-coding a replacement in a local library, which is awkward. Disable warnings/errors for deprecated functions.
Additional context
I'm trying to migrate a large amount of code from 1.4 to 1.5 to work around the security issue.
The text was updated successfully, but these errors were encountered: