Thrift: Support for explicit Python module declaration.

Summary:
Previously, Thrift used the name of the .thrift file as the python module name.
This wasn't very flexible.  Now the python module can be explicitly declared.
Also, there was no need for t_py_generator to inherit from t_oop_generator.

Reviewed By: mcslee

Test Plan:
cd test/py/explicit_module
./runtest.sh

Revert Plan: ok

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665234 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc
index 256029e..487de87 100644 (file)
--- a/compiler/cpp/src/generate/t_py_generator.cc
+++ b/compiler/cpp/src/generate/t_py_generator.cc
@@ -20,10 +20,27 @@ using namespace std;
  */
 void t_py_generator::init_generator() {
   // Make output directory
-  mkdir(T_PY_DIR, S_IREAD | S_IWRITE | S_IEXEC);
+  string module = get_real_py_module(program_);
   package_dir_ = T_PY_DIR;
-  package_dir_ = package_dir_ + "/" + program_name_;
-  mkdir(package_dir_.c_str(), S_IREAD | S_IWRITE | S_IEXEC);
+  while (true) {
+    // TODO: Do better error checking here.
+    mkdir(package_dir_.c_str(), S_IREAD | S_IWRITE | S_IEXEC);
+    std::ofstream init_py((package_dir_+"/__init__.py").c_str());
+    init_py.close();
+    if (module.empty()) {
+      break;
+    }
+    string::size_type pos = module.find('.');
+    if (pos == string::npos) {
+      package_dir_ += "/";
+      package_dir_ += module;
+      module.clear();
+    } else {
+      package_dir_ += "/";
+      package_dir_ += module.substr(0, pos);
+      module.erase(0, pos+1);
+    }
+  }
 
   // Make output file
   string f_types_name = package_dir_+"/"+"ttypes.py";
@@ -52,7 +69,7 @@ void t_py_generator::init_generator() {
     render_includes() << endl <<
     "from thrift.transport import TTransport" << endl <<
     "from thrift.protocol import fastbinary" << endl <<
-    "from thrift.protocol import TBinaryProtocol" << endl;
+    "from thrift.protocol import TBinaryProtocol" << endl << endl << endl;
 
   f_consts_ <<
     py_autogen_comment() << endl <<
@@ -68,7 +85,7 @@ string t_py_generator::render_includes() {
   const vector<t_program*>& includes = program_->get_includes();
   string result = "";
   for (size_t i = 0; i < includes.size(); ++i) {
-    result += "import " + includes[i]->get_name() + ".ttypes\n";
+    result += "import " + get_real_py_module(includes[i]) + ".ttypes\n";
   }
   if (includes.size() > 0) {
     result += "\n";
@@ -590,7 +607,8 @@ void t_py_generator::generate_service(t_service* tservice) {
 
   if (tservice->get_extends() != NULL) {
     f_service_ <<
-      "import " << tservice->get_extends()->get_program()->get_name() << "." << tservice->get_extends()->get_name() << endl;
+      "import " << get_real_py_module(tservice->get_extends()->get_program()) <<
+      "." << tservice->get_extends()->get_name() << endl;
   }
 
   f_service_ <<
@@ -1597,9 +1615,9 @@ string t_py_generator::type_name(t_type* ttype) {
   t_program* program = ttype->get_program();
   if (program != NULL && program != program_) {
     if (ttype->is_service()) {
-      return program->get_name() + "." + ttype->get_name();
+      return get_real_py_module(program) + "." + ttype->get_name();
     } else {
-      return program->get_name() + ".ttypes." + ttype->get_name();
+      return get_real_py_module(program) + ".ttypes." + ttype->get_name();
     }
   }
   return ttype->get_name();

diff --git a/compiler/cpp/src/generate/t_py_generator.h b/compiler/cpp/src/generate/t_py_generator.h
index b301d36..c367525 100644 (file)
--- a/compiler/cpp/src/generate/t_py_generator.h
+++ b/compiler/cpp/src/generate/t_py_generator.h
@@ -12,7 +12,7 @@
 #include <iostream>
 #include <vector>
 
-#include "t_oop_generator.h"
+#include "t_generator.h"
 
 #define T_PY_DIR "gen-py"
 
@@ -21,10 +21,10 @@
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
-class t_py_generator : public t_oop_generator {
+class t_py_generator : public t_generator {
  public:
   t_py_generator(t_program* program) :
-    t_oop_generator(program) {}
+    t_generator(program) {}
 
   /**
    * Init and close methods
@@ -135,6 +135,14 @@ class t_py_generator : public t_oop_generator {
   std::string type_to_enum(t_type* ttype);
   std::string type_to_spec_args(t_type* ttype);
 
+  static std::string get_real_py_module(const t_program* program) {
+    std::string real_module = program->get_py_module();
+    if (real_module.empty()) {
+      return program->get_name();
+    }
+    return real_module;
+  }
+
  private:
 
   /**

diff --git a/compiler/cpp/src/parse/t_program.h b/compiler/cpp/src/parse/t_program.h
index d516617..8420847 100644 (file)
--- a/compiler/cpp/src/parse/t_program.h
+++ b/compiler/cpp/src/parse/t_program.h
@@ -154,6 +154,14 @@ class t_program : public t_doc {
     return ruby_namespace_;
   }
 
+  void set_py_module(std::string py_module) {
+    py_module_ = py_module;
+  }
+
+  const std::string& get_py_module() const {
+    return py_module_;
+  }
+
   void set_perl_package(std::string perl_package) {
     perl_package_ = perl_package;
   }
@@ -205,6 +213,9 @@ class t_program : public t_doc {
   // Ruby namespace
   std::string ruby_namespace_;
 
+  // Python namespace
+  std::string py_module_;
+
   // Perl namespace
   std::string perl_package_;
 

diff --git a/compiler/cpp/src/thriftl.ll b/compiler/cpp/src/thriftl.ll
index d047acc..fbad955 100644 (file)
--- a/compiler/cpp/src/thriftl.ll
+++ b/compiler/cpp/src/thriftl.ll
@@ -72,6 +72,7 @@ sliteral     ("'"[^']*"'")
 "cpp_type"       { return tok_cpp_type;       }
 "java_package"   { return tok_java_package;   }
 "php_namespace"  { return tok_php_namespace;  }
+"py_module"      { return tok_py_module;      }
 "perl_package"   { return tok_perl_package;   }
 "ruby_namespace" { return tok_ruby_namespace; }
 "xsd_all"        { return tok_xsd_all;        }

diff --git a/compiler/cpp/src/thrifty.yy b/compiler/cpp/src/thrifty.yy
index 38ca516..8533805 100644 (file)
--- a/compiler/cpp/src/thrifty.yy
+++ b/compiler/cpp/src/thrifty.yy
@@ -75,6 +75,7 @@ int y_field_val = -1;
 %token tok_cpp_include
 %token tok_cpp_type
 %token tok_php_namespace
+%token tok_py_module
 %token tok_perl_package
 %token tok_java_package
 %token tok_xsd_all
@@ -269,6 +270,13 @@ Header:
         g_program->set_php_namespace($2);
       }
     }
+| tok_py_module tok_identifier
+    {
+      pdebug("Header -> tok_py_module tok_identifier");
+      if (g_parse_mode == PROGRAM) {
+        g_program->set_py_module($2);
+      }
+    }
 | tok_perl_package tok_identifier
     {
       pdebug("Header -> tok_perl_namespace tok_identifier");

diff --git a/test/py/explicit_module/runtest.sh b/test/py/explicit_module/runtest.sh
new file mode 100755 (executable)
index 0000000..8659210
--- /dev/null
+++ b/test/py/explicit_module/runtest.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+rm -rf gen-py
+../../../compiler/cpp/thrift -py test1.thrift || exit 1
+../../../compiler/cpp/thrift -py test2.thrift || exit 1
+PYTHONPATH=./gen-py python -c 'import foo.bar.baz' || exit 1
+PYTHONPATH=./gen-py python -c 'import test2' || exit 1
+PYTHONPATH=./gen-py python -c 'import test1' &>/dev/null && exit 1  # Should fail.
+cp -r gen-py simple
+../../../compiler/cpp/thrift -r -py test2.thrift || exit 1
+PYTHONPATH=./gen-py python -c 'import test2' || exit 1
+diff -ur simple gen-py > thediffs
+file thediffs | grep -s -q empty || exit 1
+rm -rf simple thediffs
+echo 'All tests pass!'

diff --git a/test/py/explicit_module/test1.thrift b/test/py/explicit_module/test1.thrift
new file mode 100644 (file)
index 0000000..316a173
--- /dev/null
+++ b/test/py/explicit_module/test1.thrift
@@ -0,0 +1,5 @@
+py_module foo.bar.baz
+
+struct astruct {
+  1: i32 how_unoriginal;
+}

diff --git a/test/py/explicit_module/test2.thrift b/test/py/explicit_module/test2.thrift
new file mode 100644 (file)
index 0000000..fd5712e
--- /dev/null
+++ b/test/py/explicit_module/test2.thrift
@@ -0,0 +1,5 @@
+include "test1.thrift"
+
+struct another {
+  1: test1.astruct something;
+}

diff --git a/thrift.el b/thrift.el
index 67485cf..5c9e75e 100644 (file)
--- a/thrift.el
+++ b/thrift.el
@@ -10,7 +10,7 @@
 (defconst thrift-font-lock-keywords
   (list
    '("#.*$" . font-lock-comment-face)  ;; perl style comments
-   '("\\<\\(include\\|struct\\|exception\\|typedef\\|cpp_namespace\\|java_package\\|php_namespace\\|ruby_namespace\\|perl_package\\|const\\|enum\\|service\\|extends\\|void\\|async\\|throws\\|optional\\|required\\)\\>" . font-lock-keyword-face)  ;; keywords
+   '("\\<\\(include\\|struct\\|exception\\|typedef\\|cpp_namespace\\|java_package\\|php_namespace\\|ruby_namespace\\|py_module\\|perl_package\\|const\\|enum\\|service\\|extends\\|void\\|async\\|throws\\|optional\\|required\\)\\>" . font-lock-keyword-face)  ;; keywords
    '("\\<\\(bool\\|byte\\|i16\\|i32\\|i64\\|double\\|string\\|binary\\|map\\|list\\|set\\)\\>" . font-lock-type-face)  ;; built-in types
    '("\\<\\([0-9]+\\)\\>" . font-lock-variable-name-face)   ;; ordinals
    '("\\<\\(\\w+\\)\\s-*(" (1 font-lock-function-name-face))  ;; functions

diff --git a/thrift.vim b/thrift.vim
index c421cf3..1e2dad1 100644 (file)
--- a/thrift.vim
+++ b/thrift.vim
@@ -30,7 +30,7 @@ syn region thriftStringDouble matchgroup=None start=+"+  end=+"+
 syn match thriftNumber "-\=\<\d\+\>" contained
 
 " Keywords
-syn keyword thriftKeyword namespace cpp_namespace java_package php_namespace ruby_namespace perl_package
+syn keyword thriftKeyword namespace cpp_namespace java_package php_namespace ruby_namespace py_module perl_package
 syn keyword thriftKeyword xsd_all xsd_optional xsd_nillable xsd_namespace xsd_attrs
 syn keyword thriftKeyword include cpp_include cpp_type const optional required
 syn keyword thriftBasicTypes void bool byte i16 i32 i64 double string binary