refactor: prefix extensions with package name

[ven-k]

• this will ensure extension names are unique
• whenever there is another package with same ext name, building sysimage fails
• ex: LogExpFunctions has its own ChainRulesCoreExt.jl

Ex: https://github.com/SciML/DiffEqBase.jl/tree/master/ext follows this for above mentioned reasons

[devmotion]

As mentioned here, I wonder if this will be fixed in Pkg before Julia 1.9 is released or if this is an official requirement documented somewhere.

[oschulz]

Oh, I had assumed that Pkg prefixes the extensions internally somehow ...

@devmotion I suggest we merge and release this for now, we can always go back to a shorter name if this gets addressed in Pkg. Ok?

[oschulz]

@ven-k , could you change ChainOfVariablesChainRulesCoreExt to ChangesOfVariablesChainRulesCoreExt?

[oschulz]

Thanks @ven-k !

[codecov]

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (1e873bc) compared to base (856eb37).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff            @@
##            master       #13   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           61        61           
=========================================
  Hits            61        61           

Impacted Files	Coverage Δ
ext/ChangesOfVariablesChainRulesCoreExt.jl	100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[devmotion]

Personally, I would not make changes to the extensions too quickly and without approval from the Pkg developers since there does not even exist an official Julia version that supports extensions yet and the design, limitations, and restrictions are still changing (see e.g. JuliaLang/julia#48533 and the PRs linked therein). Additionally, the official documentation for these extensions (https://pkgdocs.julialang.org/dev/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions)) does not use prefixes but the current setup.

[oschulz]

@devmotion but it can't hurt to choose a more distinctive extension name, right? I don't think the Pkg standard imposes any rule on how the extension modules need to be called. And if we can avoid trouble with sysimages, why not?

[devmotion]

Sure, we can change stuff 🤷 But I feel the same as @tpapp (JuliaStats/LogExpFunctions.jl#64 (comment)), since the feature is completely experimental (you can only use it if you use beta releases or compile the Julia master branch!) and still subject to changes, I think it's better to wait until things stabilize and not distribute workarounds in multiple packages if it is unclear if they are/should be fixed upstream, if they should become part of the general guidelines, or if they will be superseded by other changes/workarounds in the near future.

[oschulz]

Sure, but on the other hand change a name that users will never "see". ChangesOfVariables is now a dependency of several important packages, and if people have trouble building sysimages right now, it wouldn't cost us anything to lessen their pain. :-)

[tpapp]

@oschulz: if anyone needs it, they can just use this branch in the manifest.

The issue (for me) is making these fixes end up in a released version, which I think is premature at this point.

[oschulz]

The issue (for me) is making these fixes end up in a released version, which I think is premature at this point.

But what does it matter how the extension is called? The name is arbitrary, right?

[KristofferC]

Even if/when PackageCompiler fixes this, I do kind of it makes sense to have the "parent" package in the extension name because otherwise, it can be confusing when this module is shown in other contexts because it will be ambiguous to what extension it is referring.

And since the name doesn't matter might as well change this here and whenever the rest with the name collisions can get fixed you can change back if you prefer the other one.