libp2p / libp2p/go-libp2p

Protobuf Makefiles should work on Windows

Open
#700 2 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
6.9k
Forks
1.3k
Avg merge
13d 21h
Merged PRs (30d)
1

Description

Running our Makefiles on Windows does not work because our protoc commands use ":" as a path delimiter in the `--proto_path` parameter. For example, in https://github.com/libp2p/go-libp2p-pubsub/blob/9f04364996b415168f0e0d7e9fc82272fbed4005/pb/Makefile#L7 we have:

`protoc --proto_path=$(GOPATH)/src:. --gogofast_out=. $<`

Because `;` is the path delimiter in Windows while `:` is the Unix path delimiter the above command will not run.

Two easy fix options are:
1) Replace every instance of `:` as a path delimiter with a PATH_DELIMITER variable as [here](https://github.com/ipfs/go-ipfs/commit/228c4db6d7cea6bc323c0a0d18f88d3ed0fd7fda)
2) Restrict each use of `--proto_path` (or it's alias `-I`) to only have one path in it since `--proto_path` can be defined multiple times per `protoc` call. For example we could write:
`protoc --proto_path=$(GOPATH)/src --proto_path=. --gogofast_out=. $<`

Option 1 pros/cons:
- More self documenting about why we're separating out paths for future people who work with (or copy-paste) our Makefiles
- Clean make command output
Option 2 pros/cons:
- Will likely look cleaner in the Makefile source
- Would need comments/documentation so people continue to use one flag per path

Vote 🎉 for option 1, Vote 🚀 for option 2

Vote for your favorites and let's get fixing.

@Stebalien @djdv @raulk

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with pb/Makefile and search the repository's Makefiles for protoc uses of --proto_path or -I with colon-separated paths. Compare the PATH_DELIMITER approach in the referenced go-ipfs commit with separate path flags, then verify the affected commands work on Windows as well as Unix.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
build-system
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.