metacoq icon indicating copy to clipboard operation
metacoq copied to clipboard

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

Open JasonGross opened this issue 2 years ago • 8 comments

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

JasonGross avatar Nov 22 '22 00:11 JasonGross

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

mattam82 avatar Nov 28 '22 15:11 mattam82

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

yforster avatar Nov 28 '22 15:11 yforster

Does changing https://github.com/MetaCoq/metacoq/blob/ca64d7236c22a438496d0360385f62216ef0a2ca/template-coq/update_plugin.sh#L22-L23 with

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

work?

JasonGross avatar Dec 05 '22 17:12 JasonGross

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

yforster avatar Dec 05 '22 17:12 yforster

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

JasonGross avatar Dec 05 '22 19:12 JasonGross

I guess we can just remove the .orig file now

mattam82 avatar Jan 05 '23 11:01 mattam82

Is this still relevant?

mattam82 avatar Mar 27 '23 08:03 mattam82

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

JasonGross avatar Mar 27 '23 11:03 JasonGross