copy_from_upstream run mistake (also in 0.9.0 release)

[baentsch]

When looking into #1584 I noticed that "main" (and 0.9.0) are not idempotent to a run of copy_from_upstream on my machine: See the differences in https://github.com/open-quantum-safe/liboqs/blob/mb-fixcopyfromupstream: Compare for example the documentation of algorithm feature sets for Sphincs.

We have a test protecting against such failures, but this "silently" failed i.e., incorrectly returned OK after reporting a failure. This ought to be investigated I'd gather.

Also the Release process documentation was lacking in this regard (now fixed).

Could someone else please take a look (maybe first just confirm this finding as I'm not sure my machine is not a bit misconfigured), maybe @praveksharma or @SWilson4 ? If confirmed, we might want to add wording to the release notes of 0.9.0 that the algorithm documentation isn't "quite right": At first blush, it seems upstream Sphincs claims support for all OSs while liboqs only documents support for Linux&Darwin for optimized Sphincs... Or is this related to the limitations of our shared code in Windows? But why then doesn't copy_from_upstream reconcile this?

[SWilson4]

Thanks for catching this, Michael!

Could someone else please take a look (maybe first just confirm this finding as I'm not sure my machine is not a bit misconfigured), maybe @praveksharma or @SWilson4 ? If confirmed, we might want to add wording to the release notes of 0.9.0 that the algorithm documentation isn't "quite right": At first blush, it seems upstream Sphincs claims support for all OSs while liboqs only documents support for Linux&Darwin for optimized Sphincs... Or is this related to the limitations of our shared code in Windows? But why then doesn't copy_from_upstream reconcile this?

I ran copy_from_upstream on the same commit and got slightly different results: see here 45f8db6. I've also confirmed that my branch is idempotent under copy_from_upstream. Not sure why it would be inconsistent across environments. :/

As for the Sphincs+ inconsistency with upstream, I believe the documentation is in line with what we want: we have a patch to document the OS restriction.

Edit for clarity: by "my branch" I mean the branch I created to test this issue. My local copy of main is not idempotent under copy_from_upstream.

[baentsch]

the documentation is in line with what we want: we have a patch to document the OS restriction.

Ah, yes, something was in the back of my mind: Thanks for the pointer. But then why didn't that get applied by copy_from_upstream??

My local copy of main is not idempotent under copy_from_upstream.

Very well, err, not well -- but at least we have a confirmed issue. Just which? #1589 does not appear to apply the sphincs patch (on my machine -- it seems to work on yours). But wait: #1589 now also has a working CI run for copy_from_upstream -- and that confirms "my" run (not properly applying the sphincs patch). What's going on here? Does copy_from_upstream silently discard failing patch applications???

[bhess]

I can also confirm on two instances (MacOS, Fedora Linux) that there are non-committed changes in the docs after running copy_from_upstream.py on main:

$  ! git status | grep modified
	modified:   docs/algorithms/kem/classic_mceliece.md
	modified:   docs/algorithms/kem/classic_mceliece.yml
	modified:   docs/algorithms/sig/falcon.md
	modified:   docs/algorithms/sig/sphincs.md
	modified:   docs/algorithms/sig/sphincs.yml
$ echo $?
1

After I commit the changes and run copy_from_upstream.py again, it's idempotent. I also don't see why this should be different across environments. @SWilson4 what env do you use? Might there be some python modules missing (requirements.txt) that would skip some steps?

CI seems to give a hint to a solution why it fails there before running !git status | grep modified:

fatal: detected dubious ownership in repository at '/__w/liboqs/liboqs'
To add an exception for this directory, call:

	git config --global --add safe.directory /__w/liboqs/liboqs

[baentsch]

That error message is fixed by my latest PR (#1589).

[SWilson4]

It seems that the inconsistency between my "env" and that of @baentsch was actually due to my running copy_from_upstream with the -k switch. When running without -k, my results match Michael's. So there was no bizarre env-specific behaviour here, just the bug which will be (hopefully!) fixed in #1589.