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

Load flint #819

Merged
merged 2 commits into from
Mar 25, 2020
Merged

Load flint #819

merged 2 commits into from
Mar 25, 2020

Conversation

fingolfin
Copy link
Member

Closes #818

@fingolfin fingolfin mentioned this pull request Mar 23, 2020
@fingolfin
Copy link
Member Author

@wbhart Does this look OK to you?

@fingolfin
Copy link
Member Author

@benlorenz I guess you can now turn your Polymake.jl branch into a PR there, too?

@wbhart
Copy link
Contributor

wbhart commented Mar 23, 2020

@fingolfin It's a shame we need to do this. But given that we have to, it's ok by me.

@fieker
Copy link
Contributor

fieker commented Mar 24, 2020

unless someone cmplains by tomorrow, I shall merge

@fingolfin
Copy link
Member Author

@fieker so are those random crashes resolved, or do they still need to be debugged?

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@benlorenz
Copy link
Collaborator

I think the dlopen statements in lines 234 and 236 should also be removed now.

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@fieker fieker merged commit 912cffd into Nemocas:master Mar 25, 2020
@fingolfin fingolfin deleted the LoadFlint branch March 25, 2020 09:15
@thofma
Copy link
Member

thofma commented Mar 25, 2020

The welcoming message is screwed up because we missed to adjust version() in src/Nemo.jl.

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@thofma
Copy link
Member

thofma commented Mar 25, 2020

The local Nemo installation is not always a git repository, so there is no sha to get.

We can just ignore it for the moment.

@benlorenz
Copy link
Collaborator

The goal is to convince Polymake to do the same and then finally register Oscar

Polymake 0.3.2 with LoadFlint dependency is registered and available for installation via add. Tag will be created by tagbot very soon.

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@thofma
Copy link
Member

thofma commented Mar 25, 2020

It is an implementation detail of the package manager. Sounds rather fragile to me, also the version of Nemo is an internal state not depending on whether it is dev'ed or not.

Alternatively we remove the welcome message and the version() altogether.

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@thofma
Copy link
Member

thofma commented Mar 25, 2020

No one else prints a welcoming message so this problem does not arise.

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@fieker
Copy link
Contributor

fieker commented Mar 25, 2020 via email

@thofma
Copy link
Member

thofma commented Mar 25, 2020

If one wants to know the specific version of a package in the current environment, one just asks for Pkg.status().

We can remove versioninfo() alltogether. It has been broken for a long time.

@rfourquet
Copy link
Contributor

I think there would be a way to automatize initialize the version number (I just tested it but I might be overlooking something). It would require to add the Pkg and UUIDs packages as dependencies. Then we initialize a global constant NEMO_VERSION at compile time using Pkg.dependencies() or Pkg.installed() depending on the julia version (this seems to add about half a second of compilation), which can be shown in __init__. Again I migth misunderstand something, but I can prepare a PR if you like the idea.

@thofma
Copy link
Member

thofma commented Mar 25, 2020

What would be the condition to add the dev?

@rfourquet
Copy link
Contributor

What would be the condition to add the dev?

I'm not certain, but I observe that in some cases the version has a "+" appended which could mean "dev'ed", but it doesn't seem reliable, I will try to find out.

@rfourquet
Copy link
Contributor

Ok, so the "+" seems to be appended for packages without a version indicated in the Project.toml file. One solution apparently would be to set the version to "0.16.4+" or "0.16.4-DEV" in Project.toml in a commit after the release, which will be reflected in NEMO_VERSION. But this is still a manual step then.

@thofma
Copy link
Member

thofma commented Mar 25, 2020

This would be the a similar step that we have now. At the moment we don't change the Project.toml version to 0.16.4-DEV, since there is no recommended way to do that (see JuliaLang/Pkg.jl#351). Instead we manually change Nemo.version() manually after a step.

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 this pull request may close these issues.

6 participants