On Thu, May 17, 2018 at 10:27:18PM -0400, James McCoy wrote:
> The biggest wrinkle is that "javac -h" _only_ generates a header file if
> there are native annotations, whereas "javah" would always generate a
> header file.  This found some places where we didn't have native
> annotations even though they were needed.
> 
> It also throws a wrench in our dependency tracking.  We can't just say
> "Hey make, these .java files all generate a .h file, and libsvnjavahl
> depends on all the .h files" anymore.
> 
> I was initially going to drop the javah type from build.conf.  Since it
> looks like we'll need to explicitly list the header files we expect to
> generate, it will probably be cleaner to use the package-based javah
> stanzas instead.  That will also keep the dependencies more accurate.

I've finally cleaned things up and the attached patch works (for me) on
Linux.  gen_base.py now has one TargetJava class (based mainly on the
old TargetJavaHeaders), which describes how to generate the .class and
optionally .h files.

Since javac only produces header files when the .java file has native
code, build.conf now lists all such files in a "native" attribute to
avoid unsatisfiable dependencies.

I know there is still work to be done for Windows, and I'll need some
help with that.  As I mentioned above, multiple files are generated per
javac command, which our current ezt templates for VisualStudio don't
seem to support.  From some brief searching, the solution files do
appear to handle this, but it's not something I can readily test.

I haven't committed to trunk since I didn't want to break the Windows
builds.  I'm open to suggestions on how to move forward.

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
Index: trunk/Makefile.in
===================================================================
--- trunk/Makefile.in	(revision 1835511)
+++ trunk/Makefile.in	(working copy)
@@ -218,7 +218,6 @@
 # special compilation for files destined for javahl (i.e. C++)
 COMPILE_JAVAHL_CXX = $(LIBTOOL) $(LTCXXFLAGS) --mode=compile $(COMPILE_CXX) $(LT_CFLAGS) $(JAVAHL_INCLUDES) -o $@ -c
 COMPILE_JAVAHL_JAVAC = $(JAVAC) $(JAVAC_FLAGS)
-COMPILE_JAVAHL_JAVAH = $(JAVAH)
 COMPILE_JAVAHL_COMPAT_JAVAC = $(JAVAC) $(JAVAC_COMPAT_FLAGS)
 
 # On Mac OS X, export an env variable so that the tests can run without
@@ -393,7 +392,6 @@
 JAVADOC = @JAVADOC@
 JAVAC_FLAGS = @JAVAC_FLAGS@
 JAVAC_COMPAT_FLAGS = @JAVAC_COMPAT_FLAGS@
-JAVAH = @JAVAH@
 JAR = @JAR@
 
 JAVA_CLASSPATH=$(abs_srcdir)/subversion/bindings/javahl/src:@JAVA_CLASSPATH@
@@ -494,8 +492,8 @@
 install-static: @INSTALL_STATIC_RULES@
 
 # JavaHL target aliases
-javahl: mkdir-init javahl-java javahl-javah javahl-callback-javah javahl-remote-javah javahl-types-javah javahl-util-javah javahl-lib @JAVAHL_TESTS_TARGET@ javahl-compat
-install-javahl: javahl install-javahl-java install-javahl-javah install-javahl-lib
+javahl: mkdir-init javahl-java javahl-callback-java javahl-remote-java javahl-types-java javahl-util-java javahl-lib @JAVAHL_TESTS_TARGET@ javahl-compat
+install-javahl: javahl install-javahl-java install-javahl-lib
 javahl-compat: javahl-compat-java @JAVAHL_COMPAT_TESTS_TARGET@
 
 clean-javahl:
Index: trunk/build/generator/gen_base.py
===================================================================
--- trunk/build/generator/gen_base.py	(revision 1835511)
+++ trunk/build/generator/gen_base.py	(working copy)
@@ -900,115 +900,67 @@
   def __init__(self, name, options, gen_obj):
     TargetLinked.__init__(self, name, options, gen_obj)
     self.link_cmd = options.get('link-cmd')
-    self.packages = options.get('package-roots', '').split()
+    self.package = options.get('package')
     self.jar = options.get('jar')
     self.deps = [ ]
-
-class TargetJavaHeaders(TargetJava):
-  def __init__(self, name, options, gen_obj):
-    TargetJava.__init__(self, name, options, gen_obj)
     self.objext = '.class'
-    self.javah_objext = '.h'
     self.headers = options.get('headers')
     self.classes = options.get('classes')
-    self.package = options.get('package')
-    self.output_dir = self.headers
+    self.native = options.get('native', '')
+    self.output_dir = self.classes
+    self.headers_dir = self.headers
 
   def add_dependencies(self):
     sources = _collect_paths(self.sources, self.path)
+    native = _collect_paths(self.native, self.path)
 
+    class_pkg_list = self.package.split('.')
+    sourcepath = build_path_split(self.path)[:-len(class_pkg_list)]
+    sourcepath = build_path_join(*sourcepath)
+
     for src, reldir in sources:
       if src[-5:] != '.java':
         raise GenError('ERROR: unknown file extension on ' + src)
 
+      sfile = SourceFile(src, reldir)
+      sfile.sourcepath = sourcepath
+
       class_name = build_path_basename(src[:-5])
 
-      class_header = build_path_join(self.headers, class_name + '.h')
-      class_header_win = build_path_join(self.headers,
-                                         self.package.replace(".", "_")
-                                         + "_" + class_name + '.h')
-      class_pkg_list = self.package.split('.')
       class_pkg = build_path_join(*class_pkg_list)
       class_file = ObjectFile(build_path_join(self.classes, class_pkg,
                                               class_name + self.objext),
-                              self.when)
+                              self.compile_cmd, self.when)
       class_file.source_generated = 1
       class_file.class_name = class_name
-      hfile = HeaderFile(class_header, self.package + '.' + class_name,
-                         self.compile_cmd)
-      hfile.filename_win = class_header_win
-      hfile.source_generated = 1
-      self.gen_obj.graph.add(DT_OBJECT, hfile, class_file)
-      self.deps.append(hfile)
 
-      # target (a linked item) depends upon object
-      self.gen_obj.graph.add(DT_LINK, self.name, hfile)
+      self.gen_obj.graph.add(DT_OBJECT, class_file, sfile)
+      self.gen_obj.graph.add(DT_LINK, self.name, class_file)
+      self.deps.append(class_file)
 
+      if (src, reldir) in native:
+        class_header = build_path_join(self.headers, class_name + '.h')
+        class_header_win = build_path_join(self.headers,
+                                           self.package.replace(".", "_")
+                                           + "_" + class_name + '.h')
+        hfile = HeaderFile(class_header, self.package + '.' + class_name,
+                           self.compile_cmd)
+        hfile.filename_win = class_header_win
+        hfile.source_generated = 1
+        self.gen_obj.graph.add(DT_OBJECT, hfile, sfile)
+        self.deps.append(hfile)
 
-    # collect all the paths where stuff might get built
-    ### we should collect this from the dependency nodes rather than
-    ### the sources. "what dir are you going to put yourself into?"
-    self.gen_obj.target_dirs.append(self.path)
-    self.gen_obj.target_dirs.append(self.classes)
-    self.gen_obj.target_dirs.append(self.headers)
-    for pattern in self.sources.split():
-      dirname = build_path_dirname(pattern)
-      if dirname:
-        self.gen_obj.target_dirs.append(build_path_join(self.path, dirname))
+        # target (a linked item) depends upon object
+        self.gen_obj.graph.add(DT_LINK, self.name, hfile)
 
-    self.gen_obj.graph.add(DT_INSTALL, self.name, self)
 
-class TargetJavaClasses(TargetJava):
-  def __init__(self, name, options, gen_obj):
-    TargetJava.__init__(self, name, options, gen_obj)
-    self.objext = '.class'
-    self.lang = 'java'
-    self.classes = options.get('classes')
-    self.output_dir = self.classes
-
-  def add_dependencies(self):
-    sources = []
-    for p in self.path.split():
-      sources.extend(_collect_paths(self.sources, p))
-
-    for src, reldir in sources:
-      if src[-5:] != '.java':
-        raise GenError('ERROR: unknown file extension on "' + src + '"')
-
-      objname = src[:-5] + self.objext
-
-      # As .class files are likely not generated into the same
-      # directory as the source files, the object path may need
-      # adjustment.  To this effect, take "target_ob.classes" into
-      # account.
-      dirs = build_path_split(objname)
-      sourcedirs = dirs[:-1]  # Last element is the .class file name.
-      while sourcedirs:
-        if sourcedirs.pop() in self.packages:
-          sourcepath = build_path_join(*sourcedirs)
-          objname = build_path_join(self.classes, *dirs[len(sourcedirs):])
-          break
-      else:
-        raise GenError('Unable to find Java package root in path "%s"' % objname)
-
-      ofile = ObjectFile(objname, self.compile_cmd, self.when)
-      sfile = SourceFile(src, reldir)
-      sfile.sourcepath = sourcepath
-
-      # object depends upon source
-      self.gen_obj.graph.add(DT_OBJECT, ofile, sfile)
-
-      # target (a linked item) depends upon object
-      self.gen_obj.graph.add(DT_LINK, self.name, ofile)
-
-      # Add the class file to the dependency tree for this target
-      self.deps.append(ofile)
-
     # collect all the paths where stuff might get built
     ### we should collect this from the dependency nodes rather than
     ### the sources. "what dir are you going to put yourself into?"
-    self.gen_obj.target_dirs.extend(self.path.split())
+    self.gen_obj.target_dirs.append(self.path)
     self.gen_obj.target_dirs.append(self.classes)
+    if self.headers:
+      self.gen_obj.target_dirs.append(self.headers)
     for pattern in self.sources.split():
       dirname = build_path_dirname(pattern)
       if dirname:
@@ -1057,8 +1009,7 @@
   'apache-mod': TargetApacheMod,
   'shared-only-lib': TargetSharedOnlyLib,
   'shared-only-cxx-lib': TargetSharedOnlyCxxLib,
-  'javah' : TargetJavaHeaders,
-  'java' : TargetJavaClasses,
+  'java' : TargetJava,
   'i18n' : TargetI18N,
   'sql-header' : TargetSQLHeader,
   }
Index: trunk/build/generator/gen_make.py
===================================================================
--- trunk/build/generator/gen_make.py	(revision 1835511)
+++ trunk/build/generator/gen_make.py	(working copy)
@@ -309,6 +309,8 @@
         ezt_target.link_cmd = target_ob.link_cmd
       if hasattr(target_ob, 'output_dir'):
         ezt_target.output_dir = target_ob.output_dir
+      if hasattr(target_ob, 'headers_dir'):
+        ezt_target.headers_dir = target_ob.headers_dir
 
       # Add additional install dependencies if necessary
       if target_ob.add_install_deps:
Index: trunk/build/generator/gen_win.py
===================================================================
--- trunk/build/generator/gen_win.py	(revision 1835511)
+++ trunk/build/generator/gen_win.py	(working copy)
@@ -362,6 +362,9 @@
 
         elif isinstance(target, gen_base.TargetJavaClasses):
           classes = targetdir = self.path(target.classes)
+          headers = ''
+          if self.headers is not None:
+            headers = '-h %s' % self.quote(self.headers)
           if self.junit_path is not None:
             classes = "%s;%s" % (classes, self.junit_path)
 
@@ -375,6 +378,7 @@
 
           cbuild = ("%s -g -Xlint -Xlint:-options " +
                     per_project_flags +
+                    headers + 
                     " -target 1.8 -source 1.8 -classpath "
                     " %s -d %s "
                     " -sourcepath %s $(InputPath)") \
Index: trunk/build/generator/templates/build-outputs.mk.ezt
===================================================================
--- trunk/build/generator/templates/build-outputs.mk.ezt	(revision 1835511)
+++ trunk/build/generator/templates/build-outputs.mk.ezt	(working copy)
@@ -102,13 +102,9 @@
 [target.varname]_OBJECTS = [for target.objects][if-index target.objects first][else] [end][target.objects][end]
 [target.varname]_DEPS = $([target.varname]_HEADERS) $([target.varname]_OBJECTS)[for target.add_deps] [target.add_deps][end][for target.deps][if-index target.deps first][else] [end][target.deps][end]
 [target.name]: $([target.varname]_DEPS)
-[if-any target.headers][target.varname]_CLASS_FILENAMES =[for target.header_class_filenames] [target.header_class_filenames][end]
-[target.varname]_CLASSES =[for target.header_classes] [target.header_classes][end]
-$([target.varname]_HEADERS): $([target.varname]_CLASS_FILENAMES)
-	[target.link_cmd] -d [target.output_dir] -classpath [target.classes]:$([target.varname]_CLASSPATH) $([target.varname]_CLASSES)
-[end][if-any target.sources][target.varname]_SRC =[for target.sources] [target.sources][end]
-$([target.varname]_OBJECTS): $([target.varname]_SRC)
-	[target.link_cmd] -d [target.output_dir] -classpath [target.classes]:$([target.varname]_CLASSPATH) $([target.varname]_SRC)
+[if-any target.sources][target.varname]_SRC =[for target.sources] [target.sources][end]
+$([target.varname]_HEADERS) $([target.varname]_OBJECTS): $([target.varname]_SRC)
+	[target.link_cmd][if-any target.headers] -h [target.headers_dir][end] -d [target.output_dir] -classpath [target.classes]:$([target.varname]_CLASSPATH) $([target.varname]_SRC)
 [if-any target.jar]
 	$(JAR) cf [target.jar_path] -C [target.classes][for target.packages] [target.packages][end][end][end]
 [else][is target.type "i18n"][target.varname]_DEPS =[for target.add_deps] [target.add_deps][end][for target.objects] [target.objects][end][for target.deps] [target.deps][end]
Index: trunk/build.conf
===================================================================
--- trunk/build.conf	(revision 1835511)
+++ trunk/build.conf	(working copy)
@@ -608,16 +608,14 @@
 [javahl-java]
 type = java
 path = subversion/bindings/javahl/src/org/apache/subversion/javahl
-  subversion/bindings/javahl/src/org/apache/subversion/javahl/callback
-  subversion/bindings/javahl/src/org/apache/subversion/javahl/remote
-  subversion/bindings/javahl/src/org/apache/subversion/javahl/types
-  subversion/bindings/javahl/src/org/apache/subversion/javahl/util
-src-root = subversion/bindings/javahl/src
 sources = *.java
+native = CommitItemStateFlags.java NativeResources.java SVNClient.java
+         SVNRepos.java
 install = javahl-java
 link-cmd = $(COMPILE_JAVAHL_JAVAC)
 classes = subversion/bindings/javahl/classes
-package-roots = org
+headers = subversion/bindings/javahl/include
+package = org.apache.subversion.javahl
 
 [javahl-compat-java]
 type = java
@@ -626,10 +624,12 @@
 install = javahl-java
 link-cmd = $(COMPILE_JAVAHL_COMPAT_JAVAC)
 classes = subversion/bindings/javahl/classes
-add-deps = $(javahl_java_DEPS)
+add-deps = $(javahl_callback_java_DEPS) $(javahl_remote_java_DEPS)
+           $(javahl_types_java_DEPS) $(javahl_util_java_DEPS)
+           $(javahl_java_DEPS)
 ### Replace JAR call in INSTALL_EXTRA_JAVAHL_JAVA macro Makefile.in.
 #jar = svn-javahl.jar
-package-roots = org
+package = org.tigris.subversion.javahl
 
 [javahl-tests]
 type = java
@@ -638,10 +638,12 @@
 install = javahl-java
 link-cmd = $(COMPILE_JAVAHL_JAVAC)
 classes = subversion/bindings/javahl/classes
-package-roots = org
+package = org.apache.subversion.javhl
 ### Java targets don't do up-to-date checks yet.
 #add-deps = javahl-java
-add-deps = $(javahl_java_DEPS)
+add-deps = $(javahl_callback_java_DEPS) $(javahl_remote_java_DEPS)
+           $(javahl_types_java_DEPS) $(javahl_util_java_DEPS)
+           $(javahl_java_DEPS)
 
 [javahl-compat-tests]
 type = java
@@ -650,66 +652,60 @@
 install = javahl-java
 link-cmd = $(COMPILE_JAVAHL_COMPAT_JAVAC)
 classes = subversion/bindings/javahl/classes
-package-roots = org
+package = org.tigris.subversion.javahl
 ### Java targets don't do up-to-date checks yet.
 #add-deps = javahl-compat-java
 add-deps = $(javahl_compat_java_DEPS)
 
-[javahl-callback-javah]
-type = javah
+[javahl-callback-java]
+type = java
 path = subversion/bindings/javahl/src/org/apache/subversion/javahl/callback
 classes = subversion/bindings/javahl/classes
 headers = subversion/bindings/javahl/include
 package = org.apache.subversion.javahl.callback
 sources = *.java
-add-deps = $(javahl_java_DEPS)
-install = javahl-javah
-link-cmd = $(COMPILE_JAVAHL_JAVAH) -force
+native = UserPasswordCallback.java
+install = javahl-java
+link-cmd = $(COMPILE_JAVAHL_JAVAC)
 
-[javahl-remote-javah]
-type = javah
+[javahl-remote-java]
+type = java
 path = subversion/bindings/javahl/src/org/apache/subversion/javahl/remote
 classes = subversion/bindings/javahl/classes
 headers = subversion/bindings/javahl/include
 package = org.apache.subversion.javahl.remote
 sources = *.java
-add-deps = $(javahl_java_DEPS)
-install = javahl-javah
-link-cmd = $(COMPILE_JAVAHL_JAVAH) -force
+native = CommitEditor.java RemoteFactory.java RemoteSession.java
+         StateReporter.java
+install = javahl-java
+link-cmd = $(COMPILE_JAVAHL_JAVAC)
 
-[javahl-types-javah]
-type = javah
+[javahl-types-java]
+type = java
 path = subversion/bindings/javahl/src/org/apache/subversion/javahl/types
 classes = subversion/bindings/javahl/classes
 headers = subversion/bindings/javahl/include
 package = org.apache.subversion.javahl.types
 sources = *.java
-add-deps = $(javahl_java_DEPS)
-install = javahl-javah
-link-cmd = $(COMPILE_JAVAHL_JAVAH) -force
+native = NativeInputStream.java NativeOutputStream.java Revision.java
+         RevisionRangeList.java RuntimeVersion.java VersionExtended.java
+         Version.java
+install = javahl-java
+link-cmd = $(COMPILE_JAVAHL_JAVAC)
 
-[javahl-util-javah]
-type = javah
+[javahl-util-java]
+type = java
 path = subversion/bindings/javahl/src/org/apache/subversion/javahl/util
 classes = subversion/bindings/javahl/classes
 headers = subversion/bindings/javahl/include
 package = org.apache.subversion.javahl.util
 sources = *.java
-add-deps = $(javahl_java_DEPS)
-install = javahl-javah
-link-cmd = $(COMPILE_JAVAHL_JAVAH) -force
+native = ConfigImpl.java ConfigLib.java DiffLib.java PropLib.java
+         RequestChannel.java ResponseChannel.java SubstLib.java
+         TunnelChannel.java
+install = javahl-java
+link-cmd = $(COMPILE_JAVAHL_JAVAC)
 
-[javahl-javah]
-type = javah
-path = subversion/bindings/javahl/src/org/apache/subversion/javahl
-classes = subversion/bindings/javahl/classes
-headers = subversion/bindings/javahl/include
-package = org.apache.subversion.javahl
-sources = *.java
-add-deps = $(javahl_java_DEPS)
-install = javahl-javah
-link-cmd = $(COMPILE_JAVAHL_JAVAH) -force
-
 [libsvnjavahl]
 description = Subversion Java HighLevel binding
 type = lib
@@ -717,9 +713,9 @@
 libs = libsvn_repos libsvn_client libsvn_wc libsvn_ra libsvn_delta libsvn_diff 
        libsvn_subr libsvn_fs aprutil apriconv apr java-sdk
 sources = *.cpp jniwrapper/*.cpp
-add-deps = $(javahl_java_DEPS) $(javahl_callback_javah_DEPS)
-           $(javahl_remote_javah_DEPS) $(javahl_types_javah_DEPS)
-           $(javahl_util_javah_DEPS) $(javahl_javah_DEPS)
+add-deps = $(javahl_java_DEPS) $(javahl_callback_java_DEPS)
+           $(javahl_remote_java_DEPS) $(javahl_types_java_DEPS)
+           $(javahl_util_java_DEPS) $(javahl_java_DEPS)
 install = javahl-lib
 # need special build rule to include -I$(JDK)/include/jni.h
 compile-cmd = $(COMPILE_JAVAHL_CXX)
@@ -1630,7 +1626,7 @@
 [__JAVAHL__]
 type = project
 path = build/win32
-libs = javahl-java javahl-javah libsvnjavahl
+libs = javahl-java libsvnjavahl
 
 [__JAVAHL_TESTS__]
 type = project

Reply via email to