qitab / cl-protobufs

Common Lisp protocol buffer implementation.
MIT License
84 stars 18 forks source link

Allow setting the filename used in the protoc command #437

Closed michaeldelago closed 2 months ago

michaeldelago commented 5 months ago

This should resolve #431.

This PR makes it so setting PROTO-SOURCE-FILE specifically uses that value as the file name passed into protoc. With this change, lisp files generated from proto files will use the PROTO-SOURCE-FILE as the file_name template var that ultimately gets sent as a key to the *file-descriptors* variable.

For example, if you define a component in your asd file like (:proto-source-file "foo/bar/baz"), the resulting lisp file will contain these lines:

(cl:eval-when (:compile-toplevel :load-toplevel :execute)
(pi:add-file-descriptor #P"foo/bar/baz.proto" 'baz)
)

The upstream HEAD yields this form:

(cl:eval-when (:compile-toplevel :load-toplevel :execute)
(pi:add-file-descriptor #P"baz.proto" 'baz)
)

This is problematic because if another file contains import "foo/bar/baz.proto";, the asdf integration will fail (at validate-imports) since upstream HEAD adds #P"baz.proto" to *file-descriptors*.

Testing on SBCL 2.3.9 with Arch Linux (kernel version 6.9.3-arch1-1) passes.

additional changes

michaeldelago commented 5 months ago

Thanks for kicking off the CI

Haven't had a reason to fiddle with ABCL, hopefully this is fun to fix

Slids commented 5 months ago

Ignore the ABCL test, it's been failing.

michaeldelago commented 5 months ago

Pretty sure I addressed all review notes, thanks for sending those.

Sorry for the delay, a lot going on this and next week

Slids commented 2 months ago

internally we got these errors CheckLint found errors. Lines should be under 100 columns. (LINE-TOO-LONG) //depot/google3/third_party/lisp/cl_protobufs/asdf.lisp:139 Lines should be under 100 columns. (LINE-TOO-LONG) //depot/google3/third_party/lisp/cl_protobufs/asdf.lisp:143 Lines should be under 100 columns. (LINE-TOO-LONG) //depot/google3/third_party/lisp/cl_protobufs/asdf.lisp:156 Lines should be under 100 columns. (LINE-TOO-LONG) //depot/google3/third_party/lisp/cl_protobufs/asdf.lisp:160

Slids commented 2 months ago

Sorry it took so long, and thanks!