Skip to content

Package wycheproof tests for the PHP ecosystem#109

Merged
cpu merged 4 commits intoC2SP:mainfrom
tob-scott-a:php-packaging
Sep 2, 2025
Merged

Package wycheproof tests for the PHP ecosystem#109
cpu merged 4 commits intoC2SP:mainfrom
tob-scott-a:php-packaging

Conversation

@tob-scott-a
Copy link
Contributor

This does the absolute minimum to publish Wycheproof to the PHP ecosystem.

After merging this PR, the C2SP maintainers will need to publish this repository to Packagist to make it usable: https://packagist.org/about

Co-authored-by: Paul Kehrer <paul.l.kehrer@gmail.com>
Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tob-scott-a Is this still something you would be interested in seeing land?

It seems pretty reasonable to me (with the caveat that I have no experience with the PHP or composer ecosystems).

Perhaps once further progress is made on CI we could think of a way to smoketest the composer distribution.

@cpu cpu mentioned this pull request Mar 19, 2025
@tob-scott-a
Copy link
Contributor Author

@cpu Yes, this will be a low-friction way to ensure PHP cryptography libraries, such as PHP-ECC and phpseclib, can consume the latest Wycheproof tests for the algorithms they implement.

@cpu cpu requested a review from FiloSottile March 28, 2025 15:49
@cpu
Copy link
Member

cpu commented Sep 1, 2025

Coming back to this with fresh eyes I'm wondering if we should avoid getting into the game of per-language bindings for the test vector data for similar reasons as the ones that motivated avoiding maintaining per-language test harnesses. It's a fair amount of work, it requires per-language expertise to do well (especially over time as technologies like trusted publishers gain adoption), and the number of languages/package publishing houses is somewhat unbounded.

Is this something that could comfortably be handled in a 3rd party repo similar to https://github.com/randombit/wycheproof-rs ?

Sorry to flip-flop on this. I know the time this has been open hasn't made a great impression w.r.t the contributor experience and I hope we can do better on future PRs.

@tob-scott-a
Copy link
Contributor Author

The composer.json file lets us pull the repo in as a dependency and never need any maintenance or ops overhead. You just do:

composer require c2sp/wycherpoof

And you're done.

In order for other people to wrap the wycheproof repository, they need to muck with gitsubmodules and the like, which is a bit of a headache.

@cpu
Copy link
Member

cpu commented Sep 2, 2025

The composer.json file lets us pull the repo in as a dependency and never need any maintenance or ops overhead.

I'm convinced there's value to PHP users in having this, I'm just a bit worried it's going to scale poorly when users from other ecosystems want the same thing, or as the inevitable churn upstream creeps in and we have to adapt our side to match.

In order for other people to wrap the wycheproof repository, they need to muck with gitsubmodules and the like, which is a bit of a headache.

That's fair, but it's also going to be a bit of a headache to keep up with packagist.org and the composer ecosystem from this side.

I'd still like to hear Filippo or someone else weigh in with their thoughts, but ignoring that for the moment maybe the solution here is to ask for help? :-) If we merged this could I count on your best-effort participation over time to keep it working or is this more of a one-and-done thing from your perspective?

@cpu
Copy link
Member

cpu commented Sep 2, 2025

I'd still like to hear Filippo or someone else weigh in with their thoughts

Chatting with Filippo on this subject we're 👍 on merging. I'm going to try to pull out some guidance for CONTRIBUTING.md for the future but the main points are:

  • Like this branch, for future ecosystems we want to focus on solutions that make it easy to consume the JSON data and not on providing deser. helpers/in-memory representations of the JSON data (e.g. annotated Go/Rust structs would be out-of-scope and should be built on top of the JSON)
  • We're going to lean on the folks proposing support for their ecosystem of choice to help us maintain the machinery going forward. You've listed yourself as the point of contact in the composer metadata so I think we're squared away there.

@tob-scott-a
Copy link
Contributor Author

tob-scott-a commented Sep 2, 2025

Y'know, that's a great point. We could just drop everything but the composer.json and onboard this repo to Packagist, and then the PHP code that wraps it could be a separate community-owned repo. I'm going to drop the PHP stuff from this PR and simplify the ongoing maintenance story.

@tob-scott-a
Copy link
Contributor Author

This is now a minimum viable pull request for PHP ecosystem support :)

@cpu
Copy link
Member

cpu commented Sep 2, 2025

Thank you!

If for whatever reason exposing the files from another repo ends up being cumbersome I think we're OK with restoring the very thin PHP bits you had before. I'm likely going to add a small .go file that does similar for that ecosystem using the embed package in the coming days (we already have a go.mod from the schema linting tooling). Mainly we want to avoid having to keep more involved code in-sync with the vector structure over time.

@tob-scott-a
Copy link
Contributor Author

I think it will be fine. As long as I can pull in c2sp/wycheproof from another package, I can write the PHP code elsewhere and maintain it asynchronously from Wycheproof development.

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@cpu cpu merged commit 78acd95 into C2SP:main Sep 2, 2025
1 check passed
@tob-scott-a tob-scott-a deleted the php-packaging branch September 2, 2025 17:47
@cpu
Copy link
Member

cpu commented Sep 2, 2025

https://packagist.org/packages/c2sp/wycheproof

@tob-scott-a Is there a good packagist username for you to use as a maintainer?

This package is not auto-updated. Please set up the GitHub Hook for Packagist so that it gets updated whenever you push!

I don't think I have the required perms to handle this part. Will follow up on that separately, and until then can boop the update button upon request.

@tob-scott-a
Copy link
Contributor Author

tob-scott-a is my Packagist username. Don't worry about that GitHub Hook error message, it tends to go away after you press Update once.

@cpu
Copy link
Member

cpu commented Sep 2, 2025

tob-scott-a is my Packagist username

done

Don't worry about that GitHub Hook error message, it tends to go away after you press Update once.

Ok! I gave the button a click and it hasn't gone away yet, but I'll check back in a little while.

Thanks again for your patience/collaboration here.

@tob-scott-a
Copy link
Contributor Author

Ah. I see. When I encounter this normally, it's usually an intermittent issue due to a race condition. But for once, the error is actually correct.

Someone with admin access needs to grant the Packagist application access to the C2SP organization so that it can auto-update whenever code is pushed into the main branch.

@FiloSottile
Copy link
Member

I think I clicked the right Grant button.

@cpu
Copy link
Member

cpu commented Sep 2, 2025

Hmm. I'm still seeing the error after hitting "update" on the packagist page.

It links to this doc page that has more detailed instructions in case it's helpful.

I can also add you as a maintainer on the packagist side if you confirm your username in case that helps (?)

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.

4 participants