LTRData / DiscUtils

Utility libraries to interact with discs, filesystem formats and more
MIT License
111 stars 24 forks source link

Only use Path.Combine on netcore #4

Closed gfs closed 9 months ago

gfs commented 9 months ago

Path.Combine has more restrictive (windows specific) behavior on Net Framework which can present problems with extracting files from formats like .dmg.

For example, this attached dmg (created from a folder with two text files using hdiutil on Mac OS, GitHub doesn't allow dmgs to be directly attached so I have zipped it) causes an Argument Exception on net4.8 but not net6 - net8 with the current code, this change resolves that argument exception allowing the dmg to also be extracted by net4.8 based programs using DiscUtils.

HfsSampleUDCO.zip

See also: https://github.com/microsoft/RecursiveExtractor/pull/145

LTRData commented 9 months ago

That's a good point. Although, it will still break if someone uses the .NET Standard 2.0 version in a .NET Framework project. That is because Path.Combine does not really have a "standard" behavior for this specified in .NET Standard 2.0.

The more I think of this, we should probably go even further and only use Path.Combine in .NET/.NET Core specifically.

gfs commented 9 months ago

I wasn't sure about the netstandard behavior but I think you're correct. Probably best to just have it on net core. I can push another commit with that.

LTRData commented 9 months ago

Merged. Thanks a lot for your contribution!

gfs commented 9 months ago

🥳 No problem. Thank you for picking up the torch on discutils!

Erik-White commented 9 months ago

@LTRData Would you mind bumping the version and publishing a new nuget package please?

LTRData commented 9 months ago

@LTRData Would you mind bumping the version and publishing a new nuget package please?

Sure! That will be done either later today or tomorrow. I might have another change as well for a new version and if it seems good, I would like to include it too.