https://pantsbuild.org/ logo
Powered by
h

hundreds-father-404

04/22/2022, 11:06 PM
@fast-nail-55400 for Go Protobuf, we may want to do this TODO before 2.11 stable.
# TODO: Consider how to merge this code with existing dependency inference code.
We need the dependencies on the generated targets to show up for project introspection to work: e.g. 
dependees
and 
--changed-since
.
How come we need to run analysis on the generated code? Could it be simpler to hardcode the packages we look for?
Also sorry that I never reviewed the PR 😕 I kept snoozing it
f

fast-nail-55400

04/22/2022, 11:07 PM
1. There is no definitive list for the packages except in the generated sources. 2. The generated sources can contain import paths to other generated sources which could maybe be inferred from protobuf_source deps but easier to just run analysis.
👍 1
The TODO refers to sharing the same “scan map of import_path->Address” logic between dep inference and the codegen
h

hundreds-father-404

04/22/2022, 11:09 PM
which could maybe be inferred from protobuf_source deps but easier to just run analysis.
Okay. From a correctness standpoint, it's probably better that we do use the protobuf dependencies. For example, we have protobuf dependency inference
Given that this is an experimental backend, I don't think we need to make this be a release blocker. We have too much on our plate and this is more polish. But I would like to open an issue in a moment after i undrestand this better
There is no definitive list for the packages except in the generated sources.
For python, we hardcode looking for the owner of 
google.protobuf
. It has worked well Given that we directly run Protoc, could we hardcode the list? We can look at the GRPC field to know if that needs to be inferred
my wip docs have the list as 
<http://github.com/golang/protobuf|github.com/golang/protobuf>
, 
<http://google.golang.org/genproto|google.golang.org/genproto>
, and 
<http://google.golang.org/protobuf|google.golang.org/protobuf>
From https://github.com/golang/protobuf
This module (github.com/golang/protobuf) contains Go bindings for protocol buffers.
It has been superseded by the google.golang.org/protobuf module, which contains an updated and simplified API, support for protobuf reflection, and many other improvements. We recommend that new code use the google.golang.org/protobuf module.
(ack im being handwavey rn)
w

witty-crayon-22786

04/22/2022, 11:39 PM
a reminder that the reason we want to know provided symbols and/or packages statically is to avoid needing to run codegen in order to compute dependencies (see https://github.com/pantsbuild/pants/issues/14960 on this topic)
➕ 1
h

hundreds-father-404

04/22/2022, 11:39 PM
and https://github.com/pantsbuild/pants/pull/15237/files. Given that we only need 
<http://google.golang.org/protobuf|google.golang.org/protobuf>
, I think 1 might not be true anymore? i.e. it's safe to hardcode If users are somehow generating a old version that uses the order dependencies, they can always manually add the dependencies
is to avoid needing to run codegen in order to compute dependencies
Agreed. And for now, we work around it by not integrating into the dependency system
Powered by