-
Notifications
You must be signed in to change notification settings - Fork 163
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Removed APIs that were marked as deprecated a long time ago. Disabled by default support for path construction, assignment and appending from container types. Users can still enable this functionality by defining BOOST_FILESYSTEM_DEPRECATED. Updated docs, tests and examples accordingly.
- Loading branch information
Showing
16 changed files
with
19 additions
and
978 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks quickbook:
What is the recommended replacement?
Why wasn't
convenience.hppissuing deprecation errors, if it were to be removed?5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, the include was unnecessary, as nothing from the header was being used (FILESYSTEM_NO_DEPRECATED seems to be already defined.)
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, the other two breakages are in auto_index (which I fixed) and in Log :-)
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All symbols defined in the header were marked as deprecated. That the header itself wasn't was my omission, I guess, but there was no point in keeping an empty header after the symbols were removed.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will fix.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inspect is also now broken: boostorg/inspect#18 because it is using the now removed
normalize. Is replacing withcanoncialthe generally correct solution?5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p.normalize()is equivalent top = p.lexically_normal().canonicalis different in that it resolves symlinks and therefore requirespto exist.5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a list with the removed APIs and their replacements? The deprecation warnings used to tell us that, but they have been removed now, so it's not easy to figure out what
leaforbranch_path(ornormalize) need to be changed to.Maybe =delete-ing the functions would have been better as a transitory measure until everyone fixes them.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's been years of deprecation warnings, that's enough of a transition period.
The suggested replacements were mentioned in the warnings and the docs up until now. Now these symbols no longer exist. Everyone who hasn't updated their code can either remain on an older Boost release, or use the information from the older release to update their code.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the person who had to "update their code", except that it was other people's code, I can tell you from experience that that's not very convenient.
In fact removing the documentation in the exact moment it's needed the most is more than inconvenient, it's actively hostile.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can understand the frustration, but the line has to be drawn somewhere.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but you don't have to draw the line in the way that maximizes the frustration.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone will be frustrated anyway because for one reason or another didn't bother to update the code for years despite the warnings. Oh well.
Anyway, what do you suggest me to do?
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you don't even bother to read what I write.
The majority of code using these constructs is in maintenance mode. The original author has long stopped paying attention to it. There's been nobody to update it for years. The code breaks today, and someone needs to fix it, going by the compile errors alone. He has no other context.
My suggestions have already been given. If you delete the functions instead of removing them, the deprecation warnings will point to the suggested replacement in the compile errors.
Failing that, the documentation should have a section listing the suggested replacements.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did, but I disagreed with your reasoning.
Sure, but does that mean I can no longer remove deprecated code from Boost.Filesystem?
We've been though this, multiple times. Most recently, when we deprecated and removed support for C++03, regardless of the level of support of downstream libraries. And surely enough, someone was frustrated and had to go ahead and update those libraries, even if that meant only cleaning up their CI.
Ideally, the one who makes the change should make an effort to update the downstream within Boost. Honestly, I didn't think anyone was still using the deprecated Boost.Filesystem APIs, so I didn't check. I probably should have checked anyway, sorry about that. But on the other hand, as far as I understand, I'm not obliged do this either, especially given that there was a generous deprecation period.
If the function is deleted, there are no deprecation warnings. There's a compilation error that doesn't mention the replacement. At best, there's a comment nearby suggesting a replacement. And you can't
= deleteenums, for example.But most of all, this effectively means extending the deprecation period, which I simply don't want. This deprecated code is a burden which makes maintenance even more complicated than it already is.
Ok, I can restore the documentation. Although I don't think it'll make a difference. I consider the moment when the deprecation warnings were introduced was the time when it was most needed, not now.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, there aren't. But at least the message includes the function declaration, which is good enough (and better than nothing.)
https://godbolt.org/z/3efssaKGW
By one release; these are now errors, so they must be addressed. Deprecation warnings can be ignored for decades if the only "consumer" of code is CI and other tooling where one doesn't pay attention to successful builds.
Maybe not; people do pay attention to release notes, though, so it can't hurt to have it linked there.
5df060eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not everyone upgrade to every Boost release. And a missing function or a deleted function are basically equivalent anyway.
I have updated the docs with the description of the removed features.