qooxdoo / qooxdoo

qooxdoo - Universal JavaScript Framework
http://qooxdoo.org
Other
766 stars 261 forks source link

Generator cannot handle complex name spaces (BZ#1861) #2005

Closed qx-bug-importer closed 8 years ago

qx-bug-importer commented 15 years ago

Thomas Herchenroeder (@thron7) wrote:

Charles reported on the ML:

If you use a complex namespace like "org.qooxdoo.slc" and map it to folder structure under 'class', like

org/ qooxdoo/ slc/

the generator will only pick up the top-level part of the name space in the library scan (like "org" in this example). If you maintain multiple libraries that distinguish themselves at some level down from the top-most part (e.g. a second library with name space "org.qooxdoo.dor"), the generator will overwrite the top-level part multiple times, and the information of the various libraries is lost (for some functionality of the generator).

One consequence is that class URIs in the generated source output are wrong, and class files are searched in the wrong library.

assigned to Thomas Herchenroeder (@thron7)

qx-bug-importer commented 15 years ago

Thomas Herchenroeder (@thron7) wrote:

Generator.py patch against 0.8.1 release version

Charles, you might use this patch if you decide to use a patched 0.8.1 version of qooxdoo

Generator.py patch against 0.8.1 release version

Index: Generator.py
===================================================================
--- Generator.py    (revision 17332)
+++ Generator.py    (revision 17057)
@@ -80,18 +80,6 @@
         if not isinstance(library, types.ListType):
             return (_namespaces, _classes, _docs, _translations, _libs)

-        def getJobsLib(path):
-            lib = None
-            path = os.path.abspath(os.path.normpath(path))
-            libMaps = self._job.getFeature("library")
-            for l in libMaps:
-                if l['path'] == path:
-                    lib = l
-                    break
-            if not lib:   # this must never happen
-                raise RuntimeError("Unable to look up library \"%s\" in Job definition" % path)
-            return lib
-
         for entry in library:
             key  = entry["path"]

@@ -100,12 +88,9 @@
                 path = memcache[key]
             else:
                 path = LibraryPath(entry, self._console)
-                namespace = getJobsLib(key)['namespace']
-                path._namespace = namespace  # patch namespace
                 path.scan()

             namespace = path.getNamespace()
-            #namespace = getJobsLib(key)['namespace']
             _namespaces.append(namespace)

             classes = path.getClasses()
@@ -348,7 +333,7 @@
         self._depLoader      = DependencyLoader(self._classes, self._cache, self._console, self._treeLoader, require, use)
         self._treeCompiler   = TreeCompiler(self._classes, self._cache, self._console, self._treeLoader)
         self._locale         = Locale(self._classes, self._translations, self._cache, self._console, self._treeLoader)
-        self._partBuilder    = PartBuilder(self._console, self._depLoader, self._treeCompiler)
+        partBuilder          = PartBuilder(self._console, self._depLoader, self._treeCompiler)
         self._resourceHandler= _ResourceHandler(self)

         # Updating translation
@@ -409,7 +394,7 @@
                 # Computing packages
                 # partPackages[partId]=[0,1,3]
                 # packageClasses[0]=['qx.Class','qx.bom.Stylesheet',...]
-                partPackages, packageClasses = self._partBuilder.getPackages(partIncludes, smartExclude, classList, collapseCfg, variants, minPackageSize, minPackageSizeForUnshared)
+                parts, packages = self._partBuilder.getPackages(partIncludes, smartExclude, classList, collapseCfg, variants, minPackageSize, minPackageSizeForUnshared)

             else:
                 # Emulate configuration
qx-bug-importer commented 15 years ago

Thomas Herchenroeder (@thron7) wrote:

provided a fix for the bug (r17325)

the fix patches the _namespace member of LibraryPath objects; tests look good;

but the balance between LibraryPath objects (for library file scanning) and Job config "library" members (from config.json and Manifest.json information) has to be re-worked.

I'll appended a patch file that can be applied against the affected 0.8.1 release version of tool/pylib/generator/Generator.py

qx-bug-importer commented 15 years ago

Matthew (matthew+qooxdoo) wrote:

This fix breaks projects that use contribs :(

I don't know the generator tool that well, but I've found if I remove line 85 from generator.py it fixes the problem.

The line is: path = os.path.abspath(os.path.normpath(path))

And this is the output from my 'generate source'

Traceback (most recent call last): File "C:\Projects\web2\qooxdoo-sdk\tool\bin\generator.py", line 144, in <module> main() File "C:\Projects\web2\qooxdoo-sdk\tool\bin\generator.py", line 139, in main Generator(config, job, console).run() File "C:\Projects\web2\qooxdoo-sdk\tool\pylib\generator\Generator.py", line 344, in run self._libs) = self.scanLibrary(config.get("library")) File "C:\Projects\web2\qooxdoo-sdk\tool\pylib\generator\Generator.py", line 103, in scanLibrary namespace = getJobsLib(key)['namespace'] File "C:\Projects\web2\qooxdoo-sdk\tool\pylib\generator\Generator.py", line 92, in getJobsLib raise RuntimeError("Unable to look up library "%s" in Job definition" % path) RuntimeError: Unable to look up library "C:\Projects\web2\frontend\cache\downloads\FlowLayout\trunk" in Job definition

Also: Am I correct in reopening this bug as it broke something or should I have opened a new bug using the "Bug x depends on" or "bug x blocks"?

qx-bug-importer commented 15 years ago

Thomas Herchenroeder (@thron7) wrote:

applied Matt's fix (r17334)

qx-bug-importer commented 15 years ago

Thomas Herchenroeder (@thron7) wrote:

Updated Generator.py against 0.8.1 patch

patch containing Matt's fix

Updated Generator.py against 0.8.1 patch

--- Generator.py    2009-01-21 18:49:03.000000000 +0000
+++ Generator.py.current    2009-01-21 18:16:34.000000000 +0000
@@ -80,6 +80,18 @@
         if not isinstance(library, types.ListType):
             return (_namespaces, _classes, _docs, _translations, _libs)

+        def getJobsLib(path):
+            lib = None
+            #path = os.path.abspath(os.path.normpath(path))  # this shouldn't be necessary, and breaks in some scenarios (s. bug#1861)
+            libMaps = self._job.getFeature("library")
+            for l in libMaps:
+                if l['path'] == path:
+                    lib = l
+                    break
+            if not lib:   # this must never happen
+                raise RuntimeError("Unable to look up library \"%s\" in Job definition" % path)
+            return lib
+
         for entry in library:
             key  = entry["path"]

@@ -88,9 +100,12 @@
                 path = memcache[key]
             else:
                 path = LibraryPath(entry, self._console)
+                namespace = getJobsLib(key)['namespace']
+                path._namespace = namespace  # patch namespace
                 path.scan()

             namespace = path.getNamespace()
+            #namespace = getJobsLib(key)['namespace']
             _namespaces.append(namespace)

             classes = path.getClasses()
@@ -333,7 +348,7 @@
         self._depLoader      = DependencyLoader(self._classes, self._cache, self._console, self._treeLoader, require, use)
         self._treeCompiler   = TreeCompiler(self._classes, self._cache, self._console, self._treeLoader)
         self._locale         = Locale(self._classes, self._translations, self._cache, self._console, self._treeLoader)
-        partBuilder          = PartBuilder(self._console, self._depLoader, self._treeCompiler)
+        self._partBuilder    = PartBuilder(self._console, self._depLoader, self._treeCompiler)
         self._resourceHandler= _ResourceHandler(self)

         # Updating translation
@@ -394,7 +409,7 @@
                 # Computing packages
                 # partPackages[partId]=[0,1,3]
                 # packageClasses[0]=['qx.Class','qx.bom.Stylesheet',...]
-                parts, packages = self._partBuilder.getPackages(partIncludes, smartExclude, classList, collapseCfg, variants, minPackageSize, minPackageSizeForUnshared)
+                partPackages, packageClasses = self._partBuilder.getPackages(partIncludes, smartExclude, classList, collapseCfg, variants, minPackageSize, minPackageSizeForUnshared)

             else:
                 # Emulate configuration
qx-bug-importer commented 15 years ago

Thomas Herchenroeder (@thron7) wrote:

applied a corollary patch from Matt that normalizes the path to contrib downloads (r17339)

(this would probably re-enable my initial version of bug fix, but since it works at it is now I just leave it that way).

qx-bug-importer commented 15 years ago

Matthew (matthew+qooxdoo) wrote:

Just checked this out and it is working fine. Thanks for commiting the change :)

Bug closed

qx-bug-importer commented 15 years ago

Charles du Jeu (charledou) wrote:

It's working for me too (i'm the initial reporter) thanks a lot Charles

qx-bug-importer commented 15 years ago

Mathieu Baudier (mbaudier) wrote:

We generated a package which contains only:

It can be found here: http://www.argeo.org/maven/argeo/org/argeo/dep/dist/qooxdoo-sdk/0.8.1.argeo.1/qooxdoo-sdk-0.8.1.argeo.1-dist.zip and may be helpful to anyone who just wants to test this particular fix.

Please note that the root directory of the package is qooxdoo-sdk and not qooxdoo-sdk-0.8.1.

qx-bug-importer commented 15 years ago

Charles du Jeu (charledou) wrote:

Today I'm experimenting further in my "exotic" setup where I use java-like namespace, and handling with resources, I found out other problems.

I've done my own patch, using the same method as Thomas had when correcting the BZ#1861 (see below). However I'm not sure that it's a very good solution : shouldn't we definitely fix the namespace bug directly in the LibraryPath object, using the "namespace" provided by the Manifest.json, instead of detecting the folders??? Otherwise I'm quasi sure that this will reappear.

--- E:/charlie/software/APPS/slc/svn/server/org.argeo.slc.ria/src/qooxdoo-sdk/tool/pylib/generator/GeneratorOrig.py lun. mars 2 11:00:12 2009 +++ E:/charlie/software/APPS/slc/svn/server/org.argeo.slc.ria/src/qooxdoo-sdk/tool/pylib/generator/Generator.py mar. juin 30 21:25:10 2009 @ -1698,7 +1698,22 @

     # - Helpers -----------------------------------------------------------

Cheers Charles

qx-bug-importer commented 15 years ago

Thomas Herchenroeder (@thron7) wrote:

> + The solution : once again, digging the Generator.py (I will end up a python expert!!), I found the responsible is the Cache management inside the ResourceHandler : in the findAllResources function, the cache data is indexed by namespace. And once again as in the old bug, the namespace is still the wrong one : always only the first part of the real namespace : "org" for org.argeo.ria, "org" for org.argeo.ria.slc, and "org" again for org.argeo.ria.slcweb => 3 times copies of the image. >

Charles, thanks for making the effort. But the code has changed considerably since 0.8.1, therefore I cannot apply your patch right away. But looking through the changed lines I think we already fixed this particular issue by using the library path for the cache key, rather than the name space (this has some additional benefits, too). So it should work right away with trunk. Could you re-test your libraries with a trunk version of qooxdoo, and close this bug if it works there?! (This is generally a good strategy, to test first with trunk before opening a bug or creating a patch).

> I've done my own patch, using the same method as Thomas had when correcting the BZ#1861 (see below). However I'm not sure that it's a very good solution : shouldn't we definitely fix the namespace bug directly in the LibraryPath object, using the "namespace" provided by the Manifest.json, instead of detecting the folders??? Otherwise I'm quasi sure that this will reappear. >

You are right. Could you open a new bug for this specifically? Thanks!

qx-bug-importer commented 15 years ago

Andreas Ecker (@ecker) wrote:

This was in REOPENED state but targeting the past 0.8.2. Moved to 0.9.

Charles, could you verify this bug as fixed, please?

qx-bug-importer commented 15 years ago

Andreas Ecker (@ecker) wrote:

.

qx-bug-importer commented 14 years ago

Charles du Jeu (charledou) wrote:

Ok this is fixed in 0.8.3, sorry I didn't have time to verify it before.

qx-bug-importer commented 14 years ago

Andreas Ecker (@ecker) wrote:

Thanks Charles for verifying. Closing it for 0.8.3.