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

Run make to update template-coq/gen-src/cRelationClasses.mli.orig #791

Draft
wants to merge 1 commit into
base: coq-8.16
Choose a base branch
from

Conversation

JasonGross
Copy link
Contributor

Is this change expected? Is this file supposed to be checked in? What's going on here?

@mattam82
Copy link
Member

It looks like the extraction changed somehow? The .orig file doesn't need to be checked in, you're right.

@yforster
Copy link
Member

Can we let the patch command fail if the patch is not applicable? If not, it might make sense to keep the orig file and fail if it is not correct anymore

I hunted a problem for about an hour the other day where the issue was that extraction changed and the patch silently failed, resulting in a compilation error that I couldn't make sense of

@JasonGross
Copy link
Contributor Author

Does changing

patch -N -p0 < extraction.patch
patch -N -p0 < specFloat.patch

with

-    patch -N -p0 < extraction.patch
-    patch -N -p0 < specFloat.patch
+    patch -N -p0 < extraction.patch || exit $?
+    patch -N -p0 < specFloat.patch || exit $?

work?

@yforster
Copy link
Member

yforster commented Dec 5, 2022

Is that different to || exit? But the idea seems to be the right one to enforce a failure on a failing patch

@JasonGross
Copy link
Contributor Author

Is that different to || exit? But the idea seems to be the right one to enforce a failure on a failing patch

I think it's the same

yforster added a commit to yforster/template-coq that referenced this pull request Dec 15, 2022
@mattam82
Copy link
Member

mattam82 commented Jan 5, 2023

I guess we can just remove the .orig file now

@JasonGross JasonGross marked this pull request as draft March 2, 2023 06:06
@mattam82
Copy link
Member

Is this still relevant?

@JasonGross
Copy link
Contributor Author

The .orig file still exists, but it seems to no longer be the case that make changes it, so, idk, maybe not?

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.

3 participants