Fix SpecialFolder.MyDocuments to map to XDG properly in Unix #12575
Conversation
|
@akoeplinger, Mono maps Personal/Documents to ~ (https://github.com/mono/mono/blob/e95a1d4a00a3f8da67968d2a776be3072ce09bc7/mcs/class/corlib/System/Environment.cs#L651-L653), which is why we did the same here. Should this be changed? |
In Windows SpecialFolder.MyDocuments maps to "C:\Users\foo\My Documents". The equivalent to this in Linux should be "/home/foo/Documents", not "/home/foo". (There's already another SpecialFolder enum member that already maps to "/home/foo" and "C:\Users\foo", it is the SpecialFolder.UserProfile.)
|
Hey @stephentoub, I had a typo in my commit message (and PR description), I just corrected it.
It's a Mono bug.
I already proposed a PR for Mono but as Mono is an established platform, it could have been broken many users of the API, so they postponed the change to a major-version change. As nobody is a user of this API in dotnetCore yet (because no release contains this yet, right?), I think it should be fixed from the beginning. |
I found the PR. Sounds like it won't be changed even in a major-version change:
But part of our goal for .NET Standard is that folks can write portable libraries across, say, Mono and .NET Core, and get as consistent behavior as possible, which would include files going to the same locations when using GetFolderPath to find them. I do understand your point, I just wonder, if it's been sufficient for Mono thus far and if we want consistency across Mono and .NET Core for this kind of thing, the value of changing this in corefx may not be high enough to justify it. I'm interested in others' opinions, though. |
I thought the main idea of .NET Core was start with a clean-sheet without worrying about breaking backwards compatibility in order to do things right without constraints. I still don't get the point of .NET Standard, seems like a new way to do PCL but including .NET Core in the game, which in the end breaks the point of .NET Core itself. |
|
Personally I'd rather see this be conceptually similar across our platforms as opposed to being aligned with Mono in this case. I think it is reasonable to differ here as I would expect the negative impact to be very small- if we had already shipped CoreFX with an implementation I'd perhaps be more hesitant. |
|
This comment might only be tangentially related, but whenever I see us adding things back that have "compat caveats" like this, I tend to put them into my mental "don't use that, it's a broken abstraction" bucket of API's. When we originally started talking about bringing back a bunch more desktop API's, we had envisioned that there would be a way to warn folks about API's with legacy baggage, or at least force them to opt-in to referencing them, and their use would still be discouraged. Without that, and if we are not able to fix this sort of behavior because of mono compat, then I'm worried we will end up perpetuating buggy behavior, and potentially deteriorating trust in the BCL. Everyone would have to have their own mental bucket of stuff to avoid using for one reason or another. As an example: I have a side project that stores some configuration files in the user profile. I was thinking I would switch that code to use |
The point of .NET Core is to bring the productivity and the great language & tooling support we're used to to other platforms. Reimagining .NET in fundamental ways is considered harmful. We generally do not want to deviate in drastic ways from what the established .NET platforms have done; we will, of course, add new concepts and APIs. Sometimes, we will deviate if we think it's for the greater good, for example, mapping concepts that can't be mapped 100% across all platforms, and
.NET Standard is POSIX for .NET: it's a specification of APIs that all .NET platforms, including .NET Core, have to implement. The goal of .NET Standard is to provide meaningful foundation that all developers can rely on when writing code that has to work across multiple different flavors of .NET. In other words, .NET Standard is the codified definition of what it means to be a member of the .NET family.
In general, I agree but this may be a good case where we might want to deviate. It looks like Mono already decided that this is a bug and they will fix it in the next major update. @akoeplinger, @marek-safar: what's the timeline for such a change? Also, will Xamarin pick those up? We may want to fix this now. We generally don't want to make behavioral breaking changes but fixing bugs like this should be fair game. We could try to coordinate that change across Mono and Xamarin; but on the other hand, the earlier we fix this, the better we're all of. |
|
If it's decided in the end to merge this change, I'm thinking there could be ways to minimize the risk of this breaking change to consumers of this API: the real issue here, IMO, and why it was not caught earlier, is the enum member I suppose that it's logical to think that anyone using So the approaches that come to my mind are several, choose one or a combination of them (or/and add your idea to the list):
Completely agree with you here @mellinoe, I also have my personal mental bucket and I don't like it to grow. Having .NET opensource had the romantic idea, to me, that I could contribute now whenever I had that fear of my mental bucket growing, to fix it myself if I wanted. IMHO if this kind of things are not fixed, my mental bucket might overflow and make me want to move to another platform entirely, despite my heavy investment on it over the years. |
|
I think the clean solution would be introduction of another unique enum field (probably |
Yeah, it's limiting. If you look at the original source you'll see that we were trying to be helpful and create an alias for Making changes to the enum is a little tricky as they map directly to Also, from a .NET Standard perspective, changing the enum requires getting all of the platforms aligned with the change-whatever we add in CoreFX wouldn't be able to be added to the Standard until we get the other runtimes updated. We need to think of both what we can do in the short-term (e.g. when using .NET Standard 2.0) and longer term. My preference here is start with On the long-term end of things, I specifically changed the Windows implementation to SHGetKnownFolderPath (e.g. not
We'll probably be wanting both of these points in NetFX (MAX_PATH will likely be the real driver). That gives us a little more leverage to get a more substantive change into the Standard. If we exposed a Guid based API we would allow:
Something like: // We could potentially also have a enum based overload for the most common folders
public static string GetKnownFolder(Guid folderIdentifier);
public static class FolderGuids
{
// commonly used folder ids / concepts
} |
|
There seems to be design discussion happening here. If that's the case, please let's move it into an issue, not PR. What is the plan for this particular PR? Do we want to take it? If it needs non-trivial work that won't happen right away, let's close it for now and reopen when it is ready. |
|
@JeremyKuhne what's your recommendation here? |
|
As I stated above, I would like to see us align with NetFX, then look at introducing a more flexible, Guid based system. I've opened an issue for the new API: #12892 |
I'm not sure what it means to "align with NetFX" her, since the desktop implementation doesn't run on Unix, so there's no comparison there in terms of the value produced. The only comparison we have is what Mono does. As such I don't understand the compat caveat argument: the only compat here we could imagine would be with Mono, and the current implementation matches Mono... this PR is the one that's introducing the divergence. My main concern with this change is that someone has multiple programs on their machine, some running on .NET Core, some running on Mono, with the same .NET code (maybe even the exact same binary), yet looking for files in different locations, even though the developer has specified the same logical path in both cases. If we don't care about that, we can change take the change, but I personally think we should care. Up to you guys. |
|
ps If you do take the change, the tests will need to be fixed; they're currently broken with this change on systems that don't have the target folder. |
Conceptually I mean. The desktop doesn't make this the root User folder. Adding
I'm arguing against adding/changing enum values (not what path they map to) as a possible solution. I'll finish fleshing out my new API proposal (#12892). I think that would address the need here and allow us to stay with the Mono def for the enum. |
Ah, ok, I agree with that. I was referring to @mellinoe's comment "whenever I see us adding things back that have "compat caveats" like this, I tend to put them into my mental "don't use that, it's a broken abstraction" bucket of API's".
I see. That makes sense, though path location across OSes is going to be different for lots of things; to me the more important consistency with regards to path location is on a particular OS, which means consistency between how .NET code behaves when running on different stacks on the same OS, which means consistency between the paths used by .NET Core and Mono. Again, my $.02. |
|
To clarify, I definitely agree that we should match what mono does in this case. If they fix in their next release, we should do the same. But, it seems like it'll be pretty tricky to fix something like this, because you'd have to make everyone move all their files from one folder to another so that their apps don't break 😄 . My previous post was just lamenting the behavior at all, and trying to echo some of @knocte 's sentiment. |
|
@knocte requested this breaking change in Mono a while ago and at the reasons for rejecting are simple, it would break existing code that has been built for Unix on .NET At best, introducing this change that has different meanings between .NET Core and Mono on Unix would put the API in the avoid bucket. At worst, people would discover subtle breakage and either resort to some sort of "if (RunningOnMono)" code paths. |
@JeremyKuhne will this new API be included in .NET Standard? Will the old SpecialFolder API be marked as Obsolete() in favour of this new one? Thanks |
|
Closing per above comments -- making the fix would create incompatibility between .NET Core and Mono, which is not desired, unless there is significant value. Adding new 'good' API is tracked in #12892. |
It can be part of .NET Standard when all .NET platforms implement it (.NET Core, Mono, "full" .NET Framework). So post-.NET Standard 2.0.
Let's track it either in separate issue, or as part of #12892 proposal (that would be IMO best). |
10 participants
In Windows SpecialFolder.MyDocuments maps to "C:\Users\foo\My Documents".
The equivalent to this in Linux should be
"/home/foo/Documents", not "/home/foo".
(There's already another SpecialFolder enum member
that already maps to "/home/foo" and
"C:\Users\foo", it is the SpecialFolder.UserProfile.)