Skip to content
This repository was archived by the owner on Apr 16, 2022. It is now read-only.

Issue 824 relative paths in cli #833

Merged
merged 4 commits into from
Nov 20, 2019
Merged

Issue 824 relative paths in cli #833

merged 4 commits into from
Nov 20, 2019

Conversation

ggalmazor
Copy link
Contributor

@ggalmazor ggalmazor commented Nov 19, 2019

Closes #824

Test it with this JAR: briefcase_p833.zip

What has been done to verify that this works as intended?

  • Run the automated tests
  • Manually pulled forms from the sandbox using the CLI and providing relative paths in the -sd arg

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

Intercepting the value users provide as the storage directory while parsing CLI args feels like the smallest possible change and probably the safest place to do it.

Another approach would be to patch the particular place where the error was being produced, but that wouldn't get us rid of other similar undiscovered problems in other parts of the codebase.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

The change will allow users to provide relative paths in the CLI. It shouldn't have unexpected behavior changes for users double-clicking the JAR file or manually launching the GUI from the shell, since the GUI always works with absolute paths.

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

Nope.

We will ensure the storage directory is absolute in the arg mapper
We will use the user.dir property (the directory where the java command was launched) to resolve the arg's value if it's a relative path
We will use it in other path input args too
@ggalmazor
Copy link
Contributor Author

@chrissyhroberts, this PR should take care of relative paths in any CLI arg that takes filesystem paths: storage dir, export dir, pem file path, and the odk dir when pulling from Collect.

I've attached a new JAR in case you want to give it a try.

@ggalmazor ggalmazor added this to the v1.17.1 milestone Nov 19, 2019
@chrissyhroberts
Copy link

chrissyhroberts commented Nov 20, 2019

I've tested this and it seems to work

with v.1.17.0

2019-11-20 12:58:38,946 [main] ERROR org.opendatakit.briefcase.Launcher - Error
java.io.UncheckedIOException: java.nio.file.NoSuchFileException: Data/ODK Briefcase Storage/Data/ODK Briefcase Storage/forms/V1_01_formname/metadata.json
	at org.opendatakit.briefcase.reused.UncheckedFiles.write(UncheckedFiles.java:80)
	at org.opendatakit.briefcase.reused.UncheckedFiles.write(UncheckedFiles.java:73)
	at org.opendatakit.briefcase.model.form.FileSystemFormMetadataAdapter.serialize(FileSystemFormMetadataAdapter.java:123)
	at org.opendatakit.briefcase.model.form.FileSystemFormMetadataAdapter.persist(FileSystemFormMetadataAdapter.java:96)
	at java.base/java.util.stream.ReferencePipeline$11$1.accept(ReferencePipeline.java:441)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1654)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
	at org.opendatakit.briefcase.model.form.FileSystemFormMetadataAdapter.syncWithFilesAt(FileSystemFormMetadataAdapter.java:75)
	at org.opendatakit.briefcase.model.form.FileSystemFormMetadataAdapter.at(FileSystemFormMetadataAdapter.java:33)
	at org.opendatakit.briefcase.operations.PullFormFromAggregate.pullFormFromAggregate(PullFormFromAggregate.java:90)
	at org.opendatakit.briefcase.operations.PullFormFromAggregate.lambda$static$0(PullFormFromAggregate.java:69)
	at org.opendatakit.common.cli.Cli.lambda$run$4(Cli.java:130)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at org.opendatakit.common.cli.Cli.run(Cli.java:127)
	at org.opendatakit.briefcase.Launcher.main(Launcher.java:90)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.eclipse.jdt.internal.jarinjarloader.JarRsrcLoader.main(JarRsrcLoader.java:58)
Caused by: java.nio.file.NoSuchFileException: Data/ODK Briefcase Storage/Data/ODK Briefcase Storage/forms/V1_01_formname/metadata.json
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:116)
	at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:215)
	at java.base/java.nio.file.spi.FileSystemProvider.newOutputStream(FileSystemProvider.java:478)
	at java.base/java.nio.file.Files.newOutputStream(Files.java:219)
	at java.base/java.nio.file.Files.write(Files.java:3422)
	at org.opendatakit.briefcase.reused.UncheckedFiles.write(UncheckedFiles.java:78)
	... 23 common frames omitted

with new briefcase_p833.zip

no errors

Checked can rebuild ODK Briefcase Storage folder
Works with -sfl invoked

Ubuntu

@kkrawczyk123
Copy link
Contributor

Testes with success!
Verified on Ubuntu, MacOS and Windows.
I was able to reproduce issue on master, on pr I was not.
I have also check on regression with other pull commands - nothing weird came up.

@opendatakit-bot unlabel "needs testing"
@opendatakit-bot label "behavior verified"

@kkrawczyk123
Copy link
Contributor

@opendatakit-bot label "behavior verified"

@ggalmazor ggalmazor merged commit fd229a4 into getodk:master Nov 20, 2019
@ggalmazor ggalmazor deleted the issue_824_relative_paths_in_CLI branch November 20, 2019 14:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI can't find metadata.json file unless previously created by GUI
5 participants