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

ensure 4ti2 integration works even if user doesn't have lib4ti2_jll installed in global environment #236

Closed
fingolfin opened this issue May 23, 2020 · 11 comments · Fixed by #305

Comments

@fingolfin
Copy link
Member

The following review comment by me on PR #235 was not yet addressed; @tthsqe12 can you please look into this?

In general, lib4ti2_jll won't be installed in the main environment: If the user just did add Singular or add Oscar, then I think doing import lib4ti2_jll like that won't work (or perhaps I am missing something?).

But I think we could solve this as follows: we setup a private Julia environment for use by these scripts; that environment just has lib4ti2_jll set up; it could e.g. live in Singular.jl/deps/4ti2_env (we could either commit the relevant Project.toml and Manifest.toml, or generate them in build.jl). We'd then insert the path to the environment intro the graver etc. scripts as we generate them.

Of course other solutions are also fine.

@fieker
Copy link
Contributor

fieker commented May 23, 2020 via email

@mohamed-barakat
Copy link
Contributor

In general, lib4ti2_jll won't be installed in the main environment: If the user just did add Singular or add Oscar, then I think doing import lib4ti2_jll like that won't work (or perhaps I am missing something?).

In the situation you describe above:

  • If one invokes one of the scripts externally from the shell one indeed gets the error message (as pointed out by Claus):
ERROR: ArgumentError: Package lib4ti2_jll not found in current path:
- Run `import Pkg; Pkg.add("lib4ti2_jll")` to install the lib4ti2_jll package.

Stacktrace:
 [1] require(::Module, ::Symbol) at ./loading.jl:892
  • However, according to my understanding and experiments, the script will succeed in importing lib4ti2_jll if called by a package having it as a dependency as long as the package BinaryProvider is also among the dependencies. Please correct me if I am wrong.

@fingolfin
Copy link
Member Author

@mohamed-barakat if this is empirically true (i.e. you verified it experimentally), fine. But there is no BinaryProvider documentation other than its README, and I don't see anything to ghat effect in it (I may have missed it?). so I am still very curious about this claim. You mentioned it before, but I still didn't have time to try it.

@mohamed-barakat
Copy link
Contributor

I did the experiments again but I am now unable to justify my claim. So I now regard it as false. Sorry for the confusion.

@tthsqe12
Copy link
Contributor

If lib4ti2_jll provides us with executables, what is wrong with hard coding the paths of these executables into the three scripts for singular?

@fingolfin
Copy link
Member Author

Hardcoding breaks whenever lib4ti2_jll get updated, as the paths involve a artifact id which changes in each update. Moreover, it's not just the path to the artifact which needs to be set, but also env vars which tell the linker where to find libgmp, libglpk -- and all of these involve further artifact ids (which are sha1 checksums)

@thofma
Copy link
Collaborator

thofma commented Aug 26, 2020

Is this issue still relevant?

@fingolfin
Copy link
Member Author

yes

@fingolfin
Copy link
Member Author

Another problem with hardcoding paths of executables is that we can't just run those executables directly -- certain environment variables need to be set so that the executable find the shared libraries they use (and the correct versions of those). This is something Julia does for us in the current "scripts" we output in deps/build.jl.

One way to resolve this issue would be to regenerate these scripts whenever Singular.jl is loaded; then we could indeed write standalone shell scripts with absolute paths and which also set the environment variables mentioned above. But one has to be a tad careful about that so at to not bother people who want to run multiple instances of Singular.jl in separate julia processes (it would be annoying if one julia was loading Singular.jl which then replaces the 4ti2 wrapper scripts just at the same time as another julia process was trying to invoking that wrapper script. Luckily there are techniques to minimize the risk of that; e.g. first write the new wrapper into a temporary file, then use rename() to move that temporary file into the right place).

@thofma
Copy link
Collaborator

thofma commented Sep 11, 2020

Another issue is that Singular.jl writes stuff into the package directory of Singular_jll.jl, which is actually considered immutable. As far as I understand, we are doing this, because Singular looks for these scripts in places which are relative to the location of Singular, so we cannot put them anywhere we want?

@hannes14
Copy link
Member

These scripts must in the PATH. Singular adds the location of the binary to the PATH, so that directory is in the PATH for sure,
but others are also fine.

fingolfin added a commit to fingolfin/Singular.jl that referenced this issue Nov 25, 2020
Since we now regenerate these whenever Singular.jl is precompiled, and
thus also whenever one of the JLLs we depend on changes its artifact location,
we can now hardcode all relevant paths. That means we no longer need to
execute a separate Julia process (hence no need to worry about executing the
correct one), nor are there any issues due to lib4ti2_jll not being available
in the user environment.

Also, don't write the wrapper scripts into the Singule_jll artifact directory
(which is supposed to be read-only), but instead into the package directory
(which *also* is meant to be read-only, eventually; but for now this is slightly
better, and the code was structured in such a way that changing this later
should be straightforward).

Resolves oscar-system#236
fingolfin added a commit that referenced this issue Nov 25, 2020
Since we now regenerate these whenever Singular.jl is precompiled, and
thus also whenever one of the JLLs we depend on changes its artifact location,
we can now hardcode all relevant paths. That means we no longer need to
execute a separate Julia process (hence no need to worry about executing the
correct one), nor are there any issues due to lib4ti2_jll not being available
in the user environment.

Also, don't write the wrapper scripts into the Singule_jll artifact directory
(which is supposed to be read-only), but instead into the package directory
(which *also* is meant to be read-only, eventually; but for now this is slightly
better, and the code was structured in such a way that changing this later
should be straightforward).

Resolves #236
fingolfin added a commit to fingolfin/Singular.jl that referenced this issue Jun 6, 2023
Since we now regenerate these whenever Singular.jl is precompiled, and
thus also whenever one of the JLLs we depend on changes its artifact location,
we can now hardcode all relevant paths. That means we no longer need to
execute a separate Julia process (hence no need to worry about executing the
correct one), nor are there any issues due to lib4ti2_jll not being available
in the user environment.

Also, don't write the wrapper scripts into the Singule_jll artifact directory
(which is supposed to be read-only), but instead into the package directory
(which *also* is meant to be read-only, eventually; but for now this is slightly
better, and the code was structured in such a way that changing this later
should be straightforward).

Resolves oscar-system#236
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

Successfully merging a pull request may close this issue.

6 participants