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

Problem with several root directories containing the same version of a package #3663

Closed
ThomasBreuer opened this issue Sep 17, 2019 · 6 comments

Comments

@ThomasBreuer
Copy link
Contributor

@mohamed-barakat had observed the following problem with GAP's package loading mechanism (see oscar-system/GAP.jl#287).
When a GAP package can be found in several root directories, with the same version number, then GAP ignores all except the first instance.
It may happen that the chosen instance cannot be loaded because a compiled module is missing or has been compiled with configuration parameters that do not fit to the GAP in question (with or without Julia, say), whereas another instance would be loadable.

One could argue that GAP should load the first instance that fits, and then the current behaviour could be called a bug.

No matter whether we regard this as a bug or not, I think it would be not difficult to deal with the situation sketched above.
For that,

  • GAP must not refuse different package installations with the same version number; this concerns the function AddPackageInfos,
  • functions such as DirectoriesPackagePrograms and DirectoriesPackageLibrary must use the installation path instead of the version number as their search criterion (which is anyhow an improvement),
  • InitializePackageInfoRecords must sort the info records stably, in order to achieve that the loadable instance gets chosen in the first possible root directory.

Do you think this is sensible?

@fingolfin
Copy link
Member

Seem fine to me.

@mohamed-barakat
Copy link
Member

For me too.

@ThomasBreuer
Copy link
Contributor Author

O.k., I will create a pull request for the proposed change.

ThomasBreuer added a commit to ThomasBreuer/gap that referenced this issue Sep 19, 2019
- `AddPackageInfos` does no longer ignore package installations
  for which one instance with the same version number has already
  been found.

- `InitializePackagesInfoRecords` sorts the info records stably,
  in order to achieve that the loadable instance gets chosen
  in the first possible root directory.

- `DirectoriesPackagePrograms` and `DirectoriesPackageLibrary`
  now search according to the installation path, not to the version number.

These changes are intended to fix issue gap-system#3663.
ThomasBreuer added a commit to ThomasBreuer/gap that referenced this issue Sep 24, 2019
- `AddPackageInfos` does no longer ignore package installations
  for which one instance with the same version number has already
  been found.

- `InitializePackagesInfoRecords` sorts the info records stably,
  in order to achieve that the loadable instance gets chosen
  in the first possible root directory.

- `DirectoriesPackagePrograms` and `DirectoriesPackageLibrary`
  now search according to the installation path, not to the version number.

These changes are intended to fix issue gap-system#3663.
fingolfin pushed a commit that referenced this issue Oct 4, 2019
- `AddPackageInfos` does no longer ignore package installations
  for which one instance with the same version number has already
  been found.

- `InitializePackagesInfoRecords` sorts the info records stably,
  in order to achieve that the loadable instance gets chosen
  in the first possible root directory.

- `DirectoriesPackagePrograms` and `DirectoriesPackageLibrary`
  now search according to the installation path, not to the version number.

These changes are intended to fix issue #3663.
@DominikBernhardt
Copy link
Contributor

@ThomasBreuer For some reason this wasn't closed by merging #3668 . Should this remain open?

@ThomasBreuer
Copy link
Contributor Author

@DominikBernhardt Thanks for spotting this. No, the issue should be closed.

@mohamed-barakat
Copy link
Member

Thank you very much Thomas 👍

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

4 participants