Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 43ac21f5 authored by Cole Faust's avatar Cole Faust
Browse files

Make protobufs respect pkg_path properly 

Currently, python protobuf sources are generated as if
pkg_path didn't exist, but then are moved into the pkg_path
directory after being generated. This means they're generated
with import statements in them that don't include the pkg_path.
These import statements won't work at all when pkg_path is at
least 2 levels deep, but currently erroneously work with a 1
level deep pkg_path because we mistakenly add the top-level
modules in a soong-built python zip to the PYTHONPATH. We want
to remove those modules from the PYTHONPATH, so the generated
protobuf source files have to use the correct imports.

Since there are existing cases of code that needs to be updated,
guard this new behavior behind a flag, protos_respect_pkg_path.
We will set this to true on modules individually as we update
them, and then eventually change the default to true and remove
this flag.

Bug: 247578564
Test: m py_proto_pkg_path_test && out/host/linux-x86/nativetest64/py_proto_pkg_path_test/py_proto_pkg_path_test
Change-Id: I3695cf5521837da087592f2ad5350201035b7b0e
parent 6cbb3304

python/python.go

+28 −1
Original line number Diff line number Diff line
@@ -120,6 +120,15 @@ type BaseProperties struct { 
	// whether the binary is required to be built with embedded launcher for this actual_version.

	// this is set by the python version mutator based on version-specific properties

	Embedded_launcher *bool `blueprint:"mutated"`



	Proto struct {

		// Whether generated python protos should include the pkg_path in

		// their import statements. This is a temporary flag to help transition to

		// the new behavior where this is always true. It will be removed after all

		// usages of protos with pkg_path have been updated. The default is currently

		// false.

		Respect_pkg_path *bool

	}

}



type baseAttributes struct {
@@ -672,8 +681,26 @@ func (p *Module) createSrcsZip(ctx android.ModuleContext, pkgPath string) androi 
		protoFlags := android.GetProtoFlags(ctx, &p.protoProperties)

		protoFlags.OutTypeFlag = "--python_out"



		// TODO(b/247578564): Change the default to true, and then eventually remove respect_pkg_path

		protosRespectPkgPath := proptools.BoolDefault(p.properties.Proto.Respect_pkg_path, false)

		pkgPathForProtos := pkgPath

		if pkgPathForProtos != "" && protosRespectPkgPath {

			pkgPathStagingDir := android.PathForModuleGen(ctx, "protos_staged_for_pkg_path")

			rule := android.NewRuleBuilder(pctx, ctx)

			var stagedProtoSrcs android.Paths

			for _, srcFile := range protoSrcs {

				stagedProtoSrc := pkgPathStagingDir.Join(ctx, pkgPath, srcFile.Rel())

				rule.Command().Text("mkdir -p").Flag(filepath.Base(stagedProtoSrc.String()))

				rule.Command().Text("cp -f").Input(srcFile).Output(stagedProtoSrc)

				stagedProtoSrcs = append(stagedProtoSrcs, stagedProtoSrc)

			}

			rule.Build("stage_protos_for_pkg_path", "Stage protos for pkg_path")

			protoSrcs = stagedProtoSrcs

			pkgPathForProtos = ""

		}



		for _, srcFile := range protoSrcs {

			zip := genProto(ctx, srcFile, protoFlags, pkgPath)

			zip := genProto(ctx, srcFile, protoFlags, pkgPathForProtos)

			zips = append(zips, zip)

		}

	}

python/tests/proto_pkg_path/Android.bp

0 → 100644
+13 −0
Original line number Diff line number Diff line 
python_test_host {

    name: "py_proto_pkg_path_test",

    main: "main.py",

    srcs: [

        "main.py",

        "proto/*.proto",

    ],

    pkg_path: "mylib/subpackage",

    proto: {

        canonical_path_from_root: false,

        respect_pkg_path: true,

    },

}

python/tests/proto_pkg_path/main.py

0 → 100644
+18 −0
Original line number Diff line number Diff line 
import sys



import unittest

import mylib.subpackage.proto.test_pb2 as test_pb2

import mylib.subpackage.proto.common_pb2 as common_pb2



print(sys.path)



class TestProtoWithPkgPath(unittest.TestCase):



    def test_main(self):

        x = test_pb2.MyMessage(name="foo",

                               common = common_pb2.MyCommonMessage(common="common"))

        self.assertEqual(x.name, "foo")

        self.assertEqual(x.common.common, "common")



if __name__ == '__main__':

    unittest.main()

python/tests/proto_pkg_path/proto/common.proto

0 → 100644
+5 −0
Original line number Diff line number Diff line 
syntax = "proto3";



message MyCommonMessage {

  string common = 1;

}

python/tests/proto_pkg_path/proto/test.proto

0 → 100644
+8 −0
Original line number Diff line number Diff line 
syntax = "proto3";



import "mylib/subpackage/proto/common.proto";



message MyMessage {

  string name = 1;

  MyCommonMessage common = 2;

}