Skip to content

Conversation

marquiz
Copy link
Contributor

@marquiz marquiz commented Sep 26, 2025

Install (and use) protoc and plugins under build/tools in the source tree in an attempt to ensure that the correct version of the tooling is always used.

@marquiz marquiz force-pushed the devel/protoc-install-dir branch 4 times, most recently from bf974f8 to 450ec95 Compare September 26, 2025 08:41
@marquiz
Copy link
Contributor Author

marquiz commented Sep 26, 2025

@klihub

@klihub
Copy link
Member

klihub commented Sep 26, 2025

@marquiz I think this would be good, especially if it helps us avoiding to force dockerized protobuilds on ourselves and everybody else in a local development workflow.

One related question though. Since our scripts/install-protobuf is a verbatim copy of the one in the containerd main repo and we (try to semi-regularly) sync it with that, do you think it would be possible to contribute the same changes there, too ?

@marquiz marquiz force-pushed the devel/protoc-install-dir branch from 450ec95 to 4dfced7 Compare September 26, 2025 09:31
@marquiz
Copy link
Contributor Author

marquiz commented Sep 26, 2025

One related question though. Since our scripts/install-protobuf is a verbatim copy of the one in the containerd main repo and we (try to semi-regularly) sync it with that, do you think it would be possible to contribute the same changes there, too ?

Yeah, sure, I can try :) As soon as I get this working (i.e. make the ci pass muster).

One gap still remains that we don't check the protoc (or plugins) versions installed in the source tree.

@klihub
Copy link
Member

klihub commented Sep 26, 2025

Now I got it working locally, too. Was just a stupid user error.

@klihub klihub self-requested a review September 26, 2025 09:48
Copy link
Member

@klihub klihub left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you !

@marquiz
Copy link
Contributor Author

marquiz commented Sep 26, 2025

@klihub updated. Check it. The build from source case was broken in the previous version. I hope I got it right...

@klihub
Copy link
Member

klihub commented Sep 26, 2025

@klihub updated. Check it. The build from source case was broken in the previous version. I hope I got it right...

@marquiz It already looked good to me before you noticed that that on those architectures/path it was broken and fixed it. 🙈 😜

Install (and use) protoc and plugins under build/tools in the source
tree in an attempt to ensure that the correct version of the tooling is
always used.

Signed-off-by: Markus Lehtonen <[email protected]>
@marquiz marquiz force-pushed the devel/protoc-install-dir branch from 6528705 to 2394daa Compare September 26, 2025 13:40
@marquiz
Copy link
Contributor Author

marquiz commented Sep 29, 2025

Thanks @klihub for the review.

One possible change I had in mind is:

-INSTALL_DIR="$PWD/build/tools"
+PROTOC_DIR="$PWD/build/protoc"

That would make it completely wipe/clean protoc and deps e.g. in case of version bumps. Thoughts?

@mikebrow @chrishenzie @thaJeztah

@klihub
Copy link
Member

klihub commented Sep 29, 2025

Thanks @klihub for the review.

One possible change I had in mind is:

-INSTALL_DIR="$PWD/build/tools"
+PROTOC_DIR="$PWD/build/protoc"

That would make it completely wipe/clean protoc and deps e.g. in case of version bumps. Thoughts?

@mikebrow @chrishenzie @thaJeztah

@marquiz Sorry, you lost me there... I think. So instead of installing protoc to a common build/tools[/bin] we'd put it in a dedicated build/protoc ? But isn't #233 already doing that, IOW wiping and reinstalling in case of version bump within the same git worktree, but without installing protoc to a (more) dedicated directory ?

@marquiz
Copy link
Contributor Author

marquiz commented Sep 29, 2025

I think. So instead of installing protoc to a common build/tools[/bin] we'd put it in a dedicated build/protoc ? But isn't #233 already doing that, IOW wiping and reinstalling in case of version bump within the same git worktree, but without installing protoc to a (more) dedicated directory ?

It's like build/tools/{bin,lib,include} (and possibly even build/tools/src). #233 does not wipe anything, just copies over :/ If we install into protoc-specific path we can just rm -rf it without breaking any other tooling (that we might add in the generic tools dir in the future).

@klihub klihub merged commit 272035d into containerd:main Sep 30, 2025
16 checks passed
@marquiz marquiz deleted the devel/protoc-install-dir branch September 30, 2025 12:46
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