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

Please remove the copy of ssrZ (or rename it) #15

Open
Tracked by #143
MSoegtropIMC opened this issue Nov 4, 2022 · 13 comments
Open
Tracked by #143

Please remove the copy of ssrZ (or rename it) #15

MSoegtropIMC opened this issue Nov 4, 2022 · 13 comments
Assignees

Comments

@MSoegtropIMC
Copy link
Contributor

@strub : the change in the install path in b94f002 from CoqWord to mathcomp/word (afair it was suggested by @palmskog) has the effect that other packages which do a From mathcomp Require ssrZ now fail, because there are two ssrZ files under mathcomp:

# Error: Required library ssrZ matches several files in path (found
# /home/runner/.opam/__coq-platform.2022.09.0~dev/lib/coq/user-contrib/mathcomp/word/ssrZ.vo and
# /home/runner/.opam/__coq-platform.2022.09.0~dev/lib/coq/user-contrib/mathcomp/zify/ssrZ.vo).

This is really quite bad since ssrZ is a common thing to require.

I don't know why you copied ssrZ. If it is just for convenience / historic reasons please consider removing it and installing zify via opam. Otherwise please rename it.

In Coq Platform I am still using version 1.1 for the reason as mentioned here

@palmskog
Copy link

palmskog commented Nov 4, 2022

There is quite a lot of overlap in the content of these two files, so the ideal solution would be if both projects consolidated what they need in MathComp itself. However, since MCZify is expected to be used in more places (due to its status as "proof automation package"), maybe ssrZ in coqword could be renamed to something like ssrZutil in the meantime?

@strub
Copy link
Member

strub commented Nov 4, 2022

I don't know why you copied ssrZ.

Looking at the history of both files, assuming 2017 < 2021, should invalidate this statement.

@strub strub self-assigned this Nov 4, 2022
@strub
Copy link
Member

strub commented Nov 4, 2022

IMO, the simplest is to remove ssrZ. It may be that we can rely on mczify only. Will investigate.

@MSoegtropIMC
Copy link
Contributor Author

@strub : sorry, I should have properly investigated this - I am a bit under water with breakages in Coq Platform these days.

@palmskog
Copy link

@strub any update on plans to rely on ssrZ from mczify? Also, I would like to ping in @pi8027 for an opinion on how to best resolve this namespace collision - could some results from ssrZ in this repo be moved to mczify maybe?

@strub
Copy link
Member

strub commented Dec 12, 2022

I managed to remove nearly all the contents of thus file. I al not sure it is worth consolidating anything then.

@palmskog
Copy link

palmskog commented Dec 12, 2022

That sounds good to me, then all that may be needed is to rename the file with the remaining code (and start to depend on coq-mathcomp-zify). Please flag up when there's a new release with this content, and we'll make sure it ends up in the next Platform release.

@pi8027
Copy link

pi8027 commented Dec 12, 2022

At least I would say it would be nice to have only one source (library) of structure instances for the standard binary integer type Z. Otherwise, we may face an issue that instances provided by different libraries are not convertible. As the author of the Mczify library, I hope Mczify is the right place to provide such instances (because otherwise, Mczify has to depend on another library).

LasseBlaauwbroek added a commit to LasseBlaauwbroek/opam-coq-archive that referenced this issue Jan 26, 2023
Note: These are not my packages

Starting with `coq-mathcomp-word` `2.0`, this package lives in the namespace
`mathcomp.word`, and it includes a file `ssrZ` that conflicts with a
corresponding file from `coq-mathcomp-zify`. See
jasmin-lang/coqword#15 for the discussion upstream.

`coq-mathcomp-algebra-tactics <= 1.0.0` imports `ssrZ` while only qualifying
`mathcomp` and not `mathcomp.zify`. This has been fixed in `dev`, but is still a
problem for the released packages:
https://github.com/math-comp/algebra-tactics/blob/fb87fb0434c17e825b22e3bac9904c8e4f6cd1c2/theories/ring.v#L5

We declare a conflict with `coq-mathcomp-word.2.0` to resolve this.
LasseBlaauwbroek added a commit to LasseBlaauwbroek/apery that referenced this issue Jan 27, 2023
Without a qualified import `coq-mathcomp-apery` is incompatible with `coq-mathcomp-word`. See also jasmin-lang/coqword#15
@eponier
Copy link
Contributor

eponier commented Mar 3, 2023

What is the status of this? #17 renamed ssrZ.v into word_ssrZ.v, but the proper fix in my opinion is to remove word_ssrZ.v, as mentioned here.

@strub
Copy link
Member

strub commented Mar 3, 2023

The status is that we should implement this. #17 is a fix hack to solve the name clash issue.

@MSoegtropIMC
Copy link
Contributor Author

Of course I would prefer a proper solution. If this is not possible in say 2 weeks, I would go with the hack, so that the proper solution can be done properly.

@strub
Copy link
Member

strub commented Mar 3, 2023

I accepted your PR so that you can go with the hack.

@MSoegtropIMC
Copy link
Contributor Author

Ah yes - and there is also a new tag. So all what is missing is that you confirm in #18 that this is the tag you want to see in Coq Platform. Sorry for this rather verbose protocol - it helps to avoid mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants