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

Namespace conflict of ZString and SmartFormat.ZString packages #303

Closed
israellot opened this issue Aug 4, 2022 · 12 comments · Fixed by #309
Closed

Namespace conflict of ZString and SmartFormat.ZString packages #303

israellot opened this issue Aug 4, 2022 · 12 comments · Fixed by #309
Labels

Comments

@israellot
Copy link

If I want to use both SmartFormat and original ZString packages I get a conflict because the same types are defined in both assemblies.
This is due to the inlining of ZString code into this project.
Another solution is to use another namespace for the internal ZString implementation.

@axunonb
Copy link
Member

axunonb commented Aug 4, 2022

Hi, conflicts of namespaces can easily be resolved as described here. This could happen any time with any package.
As SmartFormat won't work without ZString, we opted for adding a specific version as part of SmartFormat that also natively supports net461, which ZStringdoesn't.

@axunonb axunonb changed the title Consider reference ZString nuget package Namespace conflict of ZString and SmartFormat.ZString packages Aug 4, 2022
@israellot
Copy link
Author

israellot commented Aug 4, 2022

I guess that's fair @axunonb , but in that case I believe it should be under a different namespace, other than Cysharp.Text , or even made internal to the assembly so that the tweaked ZString implementation does not leak outside SmartFormat's assembly. To be clear, when adding SmartFormat's reference I expect to interact with a certain public api. A public ZString api is definitely not something I would expect to part of that api.

@axunonb
Copy link
Member

axunonb commented Aug 5, 2022

Cysharp.Text , ... made internal to the assembly

How would you do that? Doesn't it actually mean to change all public classes to internal visibility?

@israellot
Copy link
Author

Precisely @axunonb, that would do the trick. Besides that, I would change prepend SmartFormat to the namespace so that all classes that the library maintains in its source are within the same root namespace. If you want, I would be happy to submit a PR.

@axunonb
Copy link
Member

axunonb commented Aug 10, 2022

Sorry for the delay. Yes, this sounds simple to accomplish with ReSharper, but would have to be applied again with new releases of ZString and thus was kind of error-prone. Do you see a way to automate in a safe manner?

@israellot
Copy link
Author

israellot commented Aug 15, 2022

I believe it could be automated as part of the build process using some tool like :

https://github.com/gluck/il-repack
https://github.com/nullean/assembly-rewriter

@axunonb
Copy link
Member

axunonb commented Aug 22, 2022

The /internalize argument of il-repack looks like what was needed. But same as LibZ and others the projects were last updated few years ago, and I wonder whether they're compatible with latest .NET Framework versions. Quite a hack after all, isn't it - considering ZString is open source?
Unfortunately, even a simple search and replace for publicto internal isn't really straight-forward.

[Update]
Just came across this blog post: Alias: An approach to .NET Assembly Conflict Resolution, pointing to a quite new GitHub project named Alias. It also was an argument to --internalize.

axunonb added a commit to axunonb/SmartFormat that referenced this issue Aug 31, 2022
… to internal

Affects: class, struct, interface, delegate, enum
axunonb added a commit that referenced this issue Aug 31, 2022
Resolves #303
Affects: class, struct, interface, delegate, enum

using https://github.com/zzzprojects/findandreplace on command line, so we can automate this step:

"fnr.exe" --cl --dir "SmartFormat.ZString\repo\src\ZString" --fileMask "*.cs,*.tt" --includeSubDirectories --caseSensitive --useRegEx --find "public (?=.*(class |struct |enum |interface |delegate ))" --replace "internal "
@MagicAndre1981
Copy link

I still got the conflict @axunonb and I think it worked for some time

image

@MagicAndre1981
Copy link

@axunonb downgrade to 3.3.0 fixed it. Comparing the versions show that #368 reverted the fix you've done here.

Why don't you add the nuget reference of ZString to your lib and consume it instead of duplicating ZString code and adding it to your lib?

@MagicAndre1981
Copy link

ok, removing ZString Package Reference from my csproj also fixes it which is totally confusing.

@axunonb
Copy link
Member

axunonb commented Jan 29, 2024

Why don't you add the nuget reference of ZString to your lib and consume it instead of duplicating ZString code and adding it to your lib?

This is still because of framework v4.6.1 not supported by Cysharp.ZString.
The automatic internalization didn't run after the last ZString update, and that's the reason for the namespace collisions your referring to.
We'll publish an updated package. Thanks for your hint.

@MagicAndre1981
Copy link

This is still because of framework v4.6.1 not supported by Cysharp.ZString.

ok, but why do you still support this? ZString has .net standard 2.0 support which makes it work with 4.6.1 (with additional system dll deployment to output folder), but 4.6.1 is out of support for nearly 2 years (April 2022).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants