XLSForm / pyxform

A Python package to create XForms for ODK Collect.
BSD 2-Clause "Simplified" License
77 stars 134 forks source link

fix windows test failures related to temp file creation by ODK Validate #541

Closed lindsay-stevens closed 2 years ago

lindsay-stevens commented 3 years ago

Resolves Windows test failure issue mentioned in #540

The error says something like "Access denied" (examples below) when ODK Validate tries to create a temp file for the purpose of processing external CSV references. The Java traceback refers to a CreateTempFile call in validate/StubReference. Test failures were constrained to those in pyxform/tests_v1/test_external_instances_for_selects.py.

Side notes: the temp file created by ODK Validate does not seem to be deleted on exit. Presumably it gets cleaned up by the OS when a user reboots or does a disk clean. Also, because of the ODK Validate error cleaning in pyxform, it wasn't obvious what the problem was, since the cleaning removes Java traceback info.

Example of the error a user sees ``` Traceback (most recent call last): File "C:\Users\lindsay\repos\pyxform\repo\pyxform\tests_v1\pyxform_test_case.py", line 202, in assertPyxformXform self._run_odk_validate(xml=xml) File "C:\Users\lindsay\repos\pyxform\repo\pyxform\tests_v1\pyxform_test_case.py", line 97, in _run_odk_validate check_xform(tmp.name) File "C:\Users\lindsay\repos\pyxform\repo\pyxform\validators\odk_validate\__init__.py", line 98, in check_xform raise ODKValidateError( pyxform.validators.odk_validate.ODKValidateError: ODK Validate Errors: java.io.IOException: Access is denied >> Something broke the parser. See above for a hint. The following files failed validation: C:\WINDOWS\Temp\tmpgtyx_f1l.xml Result: Invalid ```
Example full traceback ``` java.io.IOException: Access is denied at java.io.WinNTFileSystem.createFileExclusively(Native Method) at java.io.File.createTempFile(Unknown Source) at java.io.File.createTempFile(Unknown Source) at org.opendatakit.validate.StubReference.getResourceAsFile(StubReference.java:121) at org.opendatakit.validate.StubReference.getLocalURI(StubReference.java:83) at org.javarosa.core.reference.ReferenceManager.deriveReference(ReferenceManager.java:168) at org.javarosa.core.reference.ReferenceManager.deriveReference(ReferenceManager.java:119) at org.javarosa.core.model.instance.ExternalDataInstance.getPath(ExternalDataInstance.java:111) at org.javarosa.core.model.instance.ExternalDataInstance.parseExternalInstance(ExternalDataInstance.java:59) at org.javarosa.core.model.instance.ExternalDataInstance.build(ExternalDataInstance.java:54) at org.javarosa.xform.parse.XFormParser.parseDoc(XFormParser.java:506) at org.javarosa.xform.parse.XFormParser.parse(XFormParser.java:391) at org.javarosa.xform.parse.XFormParser.parse(XFormParser.java:370) at org.javarosa.xform.util.XFormUtils.getFormFromInputStream(XFormUtils.java:98) at org.javarosa.xform.util.XFormUtils.getFormFromInputStream(XFormUtils.java:79) at org.opendatakit.validate.FormValidator.validate(FormValidator.java:462) at org.opendatakit.validate.FormValidator.validate(FormValidator.java:420) at org.opendatakit.validate.FormValidator.validate(FormValidator.java:392) at org.opendatakit.validate.FormValidator.validateAndExitWithErrorCode(FormValidator.java:349) at org.opendatakit.validate.FormValidator.main(FormValidator.java:141) >> Something broke the parser. See above for a hint. java.lang.NullPointerException at org.opendatakit.validate.StubReference.getLocalURI(StubReference.java:83) at org.javarosa.core.reference.ReferenceManager.deriveReference(ReferenceManager.java:168) at org.javarosa.core.reference.ReferenceManager.deriveReference(ReferenceManager.java:119) at org.javarosa.core.model.instance.ExternalDataInstance.getPath(ExternalDataInstance.java:111) at org.javarosa.core.model.instance.ExternalDataInstance.parseExternalInstance(ExternalDataInstance.java:59) at org.javarosa.core.model.instance.ExternalDataInstance.build(ExternalDataInstance.java:54) at org.javarosa.xform.parse.XFormParser.parseDoc(XFormParser.java:506) at org.javarosa.xform.parse.XFormParser.parse(XFormParser.java:391) at org.javarosa.xform.parse.XFormParser.parse(XFormParser.java:370) at org.javarosa.xform.util.XFormUtils.getFormFromInputStream(XFormUtils.java:98) at org.javarosa.xform.util.XFormUtils.getFormFromInputStream(XFormUtils.java:79) at org.opendatakit.validate.FormValidator.validate(FormValidator.java:462) at org.opendatakit.validate.FormValidator.validate(FormValidator.java:420) at org.opendatakit.validate.FormValidator.validate(FormValidator.java:392) at org.opendatakit.validate.FormValidator.validateAndExitWithErrorCode(FormValidator.java:349) at org.opendatakit.validate.FormValidator.main(FormValidator.java:141) The following files failed validation: C:\WINDOWS\Temp\tmp6jmys1c7.xml Result: Invalid ```

Why is this the best possible solution? Were any other approaches considered?

Not actually sure why this works, but I have seen issues before (other projects) where not passing in certain magic env vars to a python subprocess call results in bugs or errors. Another approach is to tell Java about the temp directory with the launch argument -Djava.io.tmpdir=C:/WINDOWS/Temp.

It might be that if Java doesn't know about the temp directory at startup, it doesn't give itself adequate permissions to create files there. Maybe there is then some fall-back logic later on in CreateTempFile that tries to write to the C:/WINDOWS/Temp if the tmpdir isn't set and then that fails.

What are the regression risks?

Probably none. The fix is in a code branch that is Windows-specific (os.name == "nt"). If it causes an issue for users, they could try setting one of TMP, TEMP or TMPDIR environment variables before launching pyxform. However, in normal circumstances these variables are already set.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

I would only mention it in the docs if there is a report of this causing an issue for users. It is a bit obscure.

Before submitting this PR, please make sure you have: