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

NativeLibs version conflict #187

Closed
JJ opened this issue Jun 2, 2020 · 20 comments
Closed

NativeLibs version conflict #187

JJ opened this issue Jun 2, 2020 · 20 comments

Comments

@JJ
Copy link
Contributor

JJ commented Jun 2, 2020

Local version of NativeLibs is 0.0.3, while external version is 0.0.7. This might lead to zef trying to look for it as a dependency. Or something, I'm not really sure what. At any rate, this leads to this error. I'm not sure of how to solve this. Probably having a single one, and listing it as a reference, is much better.

@JJ
Copy link
Contributor Author

JJ commented Jun 2, 2020

Adding insult to injury, this version is listed as 0.0.3, when it's, in fact, newer than the published version of NativeLibs.
What I still don't get is why it's automatically created as a dependency if versions are not the same.

JJ added a commit that referenced this issue Jun 2, 2020
@CurtTilmes
Copy link

There are two distributions in the ecosystem that provide NativeLibs:

zef info NativeLibs
- Info for: NativeLibs
- Identity: DBIish:ver<0.5.19>
- Recommended By: Zef::Repository::Ecosystems<p6c>
- Installed: Yes
Description:     Database connectivity for Perl 6
License:         BSD-2-Clause
Source-url:      git://github.com/perl6/DBIish.git
Provides: 28 modules
Depends: 1 items

and

zef info 'NativeLibs:ver<0.0.7>:auth<github:salortiz>'
- Info for: NativeLibs:ver<0.0.7>:auth<github:salortiz>
- Identity: NativeLibs:ver<0.0.7>:auth<github:salortiz>
- Recommended By: Zef::Repository::Ecosystems<p6c>
- Installed: Yes
Description:     Native libraries utilities
Source-url:      git://github.com/salortiz/NativeLibs.git
Provides: 1 modules
Support:
#   source:     https://github.com/salortiz/NativeLibs.git
#   email:      [email protected]
Depends: 0 items

If you don't qualify, a simple use NativeLibs will get the one in DBIish (which has a higher version than the one in the plain NativeLibs distribution).

If you want the other, you have qualify more, such as use NativeLibs:ver<0.0.7>:auth<github:salortiz>

@ugexe
Copy link
Member

ugexe commented Jun 2, 2020

Generally we think of module resolution without any qualifier as taking the highest version available, but really its the highest version available from the first repo of $*REPO.repo-chain containing any version at all (I probably would have prefered considering all repos, but the current strategy is superior for speed purposes). This means when using CompUnit::Repository::FileSystem to test and there are two dependencies depend on MyDependency from different auths you get something like

raku -I /mods/MyDependency1 -I /mods/Foo -I /mods/MyDependency2 -I /mods/Bar t/my-test.t

and since each one of those -I is a separate CURFS any unqualified use of MyDependency will load whatever version is in /mods/MyDependency1 and never /mods/MyDependency2.

Technically zef could work-around this issue by first installing everything into a temporary repo but that is A) a pretty big workflow change and B) just creates a new impediance between already-installed modules and the temporary repo (since again, first $*REPO containing any version). The real way to fix this, as @CurtTilmes mentions, is to use a more qualified name when using modules. For instance DBIish could add an auth field to its META6.json and then do

- use NativeLibs;
+ use NativeLibs:auth($?DISTRIBUTION.meta<auth>); # :ver($?DISTRIBUTION.meta<version>) etc

to ensure it always uses the expected NativeLibs.

@rbt
Copy link
Collaborator

rbt commented Jun 2, 2020

Removing NativeLibs from DBIish and depending on the other package appears to work.

https://github.com/raku-community-modules/DBIish/compare/rbt.nativelibs-dependency
https://travis-ci.org/github/raku-community-modules/DBIish/builds/693835962

Tests pass on Linux for MySQL, MariaDB, SQLlite, and PostgreSQL.

If someone could test Oracle or with Windows, then removing it from DBIish might be best.

@ugexe
Copy link
Member

ugexe commented Jun 2, 2020

Some more context from 2016 -- salortiz/JsonC#2 (comment)

@tbrowder
Copy link
Contributor

tbrowder commented Jun 2, 2020

All other things being equal, would it have helped if the NativeLibs module were in CPAN rather than the ecosystem?

@ugexe
Copy link
Member

ugexe commented Jun 2, 2020

All other things being equal, would it have helped if the NativeLibs module were in CPAN rather than the ecosystem?

Nope.

It’s just inevitable that people will need to qualify their use statements more and more as time goes on.

@tbrowder
Copy link
Contributor

tbrowder commented Jun 2, 2020

Nick, I think @JJ is working on some module best practice stuff and your advice above sounds like something important to be in there. Some questions, please:

  • Why did you comment out the <ver>? Is it just not needed in this case?
  • What would be the effect if you used the same syntax for all the legal adverbs for a module without investigating what is actually being used?

@JJ
Copy link
Contributor Author

JJ commented Jun 2, 2020

This is what @tbrowder is talking about Raku/problem-solving#72
I'd be grateful if whoever can do it, would check it out and suggest changes and/or approve.

@ugexe
Copy link
Member

ugexe commented Jun 2, 2020

Why did you comment out the ? Is it just not needed in this case?

It is not needed in this case. The comment is there to show how one could add the additional constraints based on META6.json if they wanted.

What would be the effect if you used the same syntax for all the legal adverbs for a module without investigating what is actually being used?

I don't understand this question

@JJ
Copy link
Contributor Author

JJ commented Jun 2, 2020 via email

@JJ
Copy link
Contributor Author

JJ commented Jun 2, 2020

I think that for the time being the best way to solve the conflict is simply give it different names.

@tbrowder
Copy link
Contributor

tbrowder commented Jun 2, 2020

What would be the effect if you used the same syntax for all the legal adverbs for a module without investigating what is actually being used?

I don't understand this question

Should I (or could I) as a module author publishing public modules use, as a general practice, your recommendation from above and generalize it, e.g.,

use Foo:auth($DISTRIBUTION.meta<auth>):ver($DISTRIBUTION.meta<version>):api($DISTRIBUTION.meta<api>);

unless we have a special need to be more specific.

Wouldn't that take care of the current problem even if it is using more constraints?

@ugexe
Copy link
Member

ugexe commented Jun 2, 2020

Generally yes, that is what most people want. Note though that on windows this only works on releases after 2020.01 -- rakudo/rakudo@13c5d04

@tbrowder
Copy link
Contributor

tbrowder commented Jun 2, 2020

Great! I'll try to start doing that. Thanks.

@rbt
Copy link
Collaborator

rbt commented Jun 24, 2020

Is this resolved sufficiently? Can we close it?

@JJ
Copy link
Contributor Author

JJ commented Jun 25, 2020 via email

@rbt rbt closed this as completed Jun 25, 2020
@salortiz
Copy link
Contributor

The NativeLib included in DBDIsh is newer than the external one only because I back-ported from the former the cannon-name sub in 241b7d4. ( @rbt change some indentation in 0d32515)

And the only reason for not removing it from DBDIsh and using the external was, at that time, to avoid introducing in Rakudo Star another dependency.

When the external (grow-up) NativeLib was originally released, I deliberately give it a gap in its version number, so projects requiring the extended/expanded functionality can request it by an explicit "depends" : [ "NativeLibs:ver<0.0.6+>" ] in its META6.json.

So, @JJ 's ec857b4 commit is a really bad idea.

I think that removing from DBDIsh the striped down NativeLibs should work without problems.

@JJ
Copy link
Contributor Author

JJ commented Jul 14, 2020

@salortiz it's simply locking a specific version; @ugexe's idea of using the distribution version is very probably a better idea. I didn't want to remove a large chunk of code (and introducing another dependency on the process). I mean, at the end of the day, using the same name in two different (although related) pieces of code is bound to create name clashes. I'll reopen anyway and see if we can come together to a best solution (or, even better, PR)

@JJ JJ reopened this Jul 14, 2020
@softmoth
Copy link

softmoth commented Jul 14, 2020

I didn't want to remove a large chunk of code

The diff you posted on Jun 2 shows that there are no real changes. That's all extra features in the real distribution, so it is totally fine to remove it from DBIish and add the dependency, IMO.

rbt added a commit that referenced this issue Jul 15, 2020
This removes the built in NativeLibs:auth<sortiz>:ver<0.0.8> and adds a dependency on the one distributed separately as per discussion in #187.
rbt added a commit that referenced this issue Jul 16, 2020
This reverts commit ec857b4.

By bumping the version above the other NativeLibs package, zef is installing DBIish to meet a dependency requirement on that file which probably isn't what is intended by 3rd parties.

This will be followed up by using the external NativeLibs package.
rbt added a commit that referenced this issue Jul 16, 2020
This removes the built in NativeLibs:auth<sortiz>:ver<0.0.8> and adds a dependency on the one distributed separately as per discussion in #187.

Removal of :auth will be done when (if?) zef stops picking up older DBIish versions for a NativeLibs dependency.
@rbt rbt closed this as completed Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants