Project

Profile

Help

Bug #4415

closed

add_package method not implemented

Added by O'Neil Delpratt almost 5 years ago. Updated almost 3 years ago.

Status:
Closed
Priority:
Normal
Category:
Python
Start date:
2019-12-24
Due date:
% Done:

100%

Estimated time:
Applies to branch:
Fix Committed on Branch:
Fixed in Maintenance Release:
Found in version:
1.2.1
Fixed in version:
11.1
SaxonC Languages:
SaxonC Platforms:
SaxonC Architecture:

Description

The add_package method not implemented. It has a TODO note.

Actions #1

Updated by O'Neil Delpratt almost 5 years ago

  • Found in version set to 1.2.1
Actions #2

Updated by O'Neil Delpratt almost 5 years ago

The add package method now looks like the following:

     def add_package(self, list file_names):
        """
        add_package(self, file_name)
        Add an XSLT 3.0 package to the library.

        Args:
            file_name (str): The file name of the XSLT package

        """
        len_ = len(file_names)
        cdef const char* c_string
        cdef const char ** cfile_names = self.thisxptr.createCharArray(len_)
        for x in range(len_):
          c_string = make_c_str(file_names[x])
          cfile_names[x] = c_string
        self.thisxptr.addPackages(cfile_names, len_)

For this to work we I have had to add the method createCharArray in the Xslt30Processor class.

    /**
    * Utility method for working with Saxon/C on Python
    */
    XdmValue ** createXdmValueArray(int len){
	return (new XdmValue*[len]);
    }
Actions #3

Updated by Martin Honnen almost 5 years ago

The function name and the comment to explain the function signature doesn't seem to match the code, the function name add_package seems to suggest it is called for a single package, the parameter list file_names suggests it is called (once?) with a list of packages.

Actions #4

Updated by O'Neil Delpratt almost 5 years ago

  • Status changed from New to In Progress

Since this function never worked before I have decided to change the signature of the function. It now matches up with the C++ API to accept a list of file names of the XSLT packages:

add_packages(self, list file_names)
Actions #5

Updated by Martin Honnen almost 5 years ago

Instead of trying to bridge Python and C/C++ I have first tried to use addPackages from C++ but somehow I always get an error "XTSE3000: Cannot find package http://example.com/packages/ex2 (version *)".

To get no run-time error I first had to fix the JNI signature to

     void Xslt30Processor::addPackages(const char ** fileNames, int length){
              	if(exceptionOccurred()) {
              		//Possible error detected in the compile phase. Processor not in a clean state.
              		//Require clearing exception.
              		return;
              	}

              	if(length<1){

              		return;
              	}

              	jmethodID apmID =
              			(jmethodID) SaxonProcessor::sxn_environ->env->GetMethodID(cppClass,
              					"addPackages",
              					"(Ljava/lang/String;[Ljava/lang/String;)V");
              	if (!apmID) {
              		std::cerr << "Error: "<<getDllname() << "addPackages" << " not found\n"
              				<< std::endl;

              	} else {

              	 jobjectArray stringArray = NULL;

                 jclass stringClass = lookForClass(SaxonProcessor::sxn_environ->env,
                 				"java/lang/String");


                 stringArray = SaxonProcessor::sxn_environ->env->NewObjectArray((jint) length,
                 					stringClass, 0);

                 for (int i=0; i<length; i++) {

                 SaxonProcessor::sxn_environ->env->SetObjectArrayElement(stringArray, i,
                 						SaxonProcessor::sxn_environ->env->NewStringUTF(fileNames[i]));
                 }

              	SaxonProcessor::sxn_environ->env->CallObjectMethod(cppXT, apmID,
              								SaxonProcessor::sxn_environ->env->NewStringUTF(cwdXT.c_str()), stringArray);

                proc->checkAndCreateException(cppClass);
              }
              	return;

        }

In the C++ code to use the method I tried three different ways, not setting any cwd but hoping the implementation would pick the current working directory:

	const char * packagesList[] = { "add-atts-p1.xsl", "add-atts-p2.xsl" };
	trans->addPackages(packagesList, 2);

or setting the cwd or even using full file paths for all file arguments, but I always got the same error mentioned above.

The XSLT code seems fine and when I manage to compile the Transform.c in the command folder and run it with the proper options lib the code also works when called from C, although I think that way simply net.sf.saxon.Transform is called from C, not the C++ API.

Actions #6

Updated by Martin Honnen almost 5 years ago

I think part of the problem is a lack of code in https://dev.saxonica.com/repos/archive/opensource/latest9.9/hej/net/sf/saxon/option/cpp/Xslt30Processor.java, in the compilePackages method is simply calls PackageLibrary library = new PackageLibrary(compiler.getUnderlyingCompilerInfo(), packagesToLoad); but I guess that doesn't register the create library package with the compiler, so a call compiler.getUnderlyingCompilerInfo().setPackageLibrary(library); there or some return of the created PackageLibrary to be registered in the calling method is necessary.

So whatever I do in the C++ or even the Python API will not work unless that Java bridge code is fixed to ensure the compiler not only compiles but also includes the package library.

Actions #7

Updated by O'Neil Delpratt almost 5 years ago

Thanks Martin.

We have 2 main bug here.

  1. Incorrect JNI code.
  2. The following code missing in the method compilePackages of the Java API: compiler.getUnderlyingCompilerInfo().setPackageLibrary(library)

I have fixed both. Unfortunately 2. requires a new Saxon/C build. I will look to do a new maintenance release in the early part of the new year. I hope that will be ok?

Actions #8

Updated by Martin Honnen almost 5 years ago

Unfortunately 2. requires a new Saxon/C build. I will look to do a new maintenance release in the early part of the new year. I hope that will be ok?

Yes, sure, whatever suits your normal working schedule. Don't let my findings spoil your holiday season.

I guess XsltProcessor.java suffers from the same problem of not having compiler.getUnderlyingCompilerInfo().setPackageLibrary(library).

Actions #9

Updated by O'Neil Delpratt almost 5 years ago

Martin Honnen wrote:

I guess XsltProcessor.java suffers from the same problem of not having compiler.getUnderlyingCompilerInfo().setPackageLibrary(library).

Yes indeed. Fixed too.

Actions #10

Updated by O'Neil Delpratt almost 5 years ago

Revisiting this issue:

After some testing of the new packages support in Saxon/C it seems like the best way to supply packages is via configuration file. This is due to changes in the Java and dotnet product to deprecate the method compilePackages. It is still possible to save a package, but users can only now use the configuration file for XSLT packages.

Actions #11

Updated by O'Neil Delpratt almost 5 years ago

Configuration file currently not supported in Python. Investigating issue. Reported in separate bug issue: #4418

Actions #12

Updated by O'Neil Delpratt almost 5 years ago

  • Status changed from In Progress to Rejected

Marking this bug issue as rejected as it has been addressed in #4418

Actions #13

Updated by O'Neil Delpratt almost 5 years ago

  • Status changed from Rejected to In Progress

This bug issue should not have been marked as rejected. We have now removed the add_packages function. As mentioned in comment #10 use configuration file to add packages.

Actions #14

Updated by O'Neil Delpratt almost 5 years ago

  • Status changed from In Progress to Resolved
Actions #15

Updated by O'Neil Delpratt almost 3 years ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Fixed in version set to 11.1

Bug fix patched in SaxonC 11.1 release

Please register to edit this issue

Also available in: Atom PDF