Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specifying a directory of .java files as source-file src no longer works #564

Closed
ippeiukai opened this issue Nov 18, 2018 · 5 comments
Closed
Labels

Comments

@ippeiukai
Copy link

From apache/cordova#47
And originally #547 (comment)

Specifying a directory with many .java files as source-file src works with cordova-android@6, but not with cordova-android@7. (Source files would be put into incorrect subdirectory on cordova-android@latest.)

This has affected at least one plugin, as shown in following PR:
vaenow/cordova-plugin-app-update#119

This behaviour is undocumented, but has been already know on the web.
As discussed in apache/cordova#47 (comment), we are keeping this behaviour.

@brodycj
Copy link
Contributor

brodycj commented Nov 18, 2018

Thanks @ippeiukai for reporting. I cannot promise when I or anyone else will get a chance to fix this. PR would be welcome for review.

@dpogue
Copy link
Member

dpogue commented Nov 18, 2018

I'm not really a fan of this and don't really think we should support this. It was always intended to to operate on files (hence the source-file name). We need to know what types of files are being installed in order to know where they need to go in the project structure.

Of course, now it's basically impossible to drop support for this without breaking an unknown number of plugins 😞

@brodycj
Copy link
Contributor

brodycj commented Nov 19, 2018

I am also not in love with this feature, for reasons I gave in the description of apache/cordova#47. But yes the idea of breaking an unknown number of plugins without proper investigation and advance warning is pretty bad.

Assuming that this undocumented feature was broken in cordova-android 7.0.0, and we did not hear much until recently, I would now expect the number of plugins broken by this issue to be pretty minimal. I would favor deprecating this feature in the upcoming major release, with a warning message, and removing it at some point in the future. This should be part of the discussion in apache/cordova#47.

@ippeiukai
Copy link
Author

ippeiukai commented Nov 19, 2018

I’m not fun of the whole source-file element situation to be honest.
I feel it is already compromised when it allowed any file to be copied into the target dir inside Cordova project. It’s not just directory as src feature.
To me, specifying a directory of .java files is far more aligned with the purpose of source-file element than specifying individual .so or .a files. But that’s matter of personal opinion. My point is that it’s not just the directory that should be deprecated.

The documentation on the target-dir attribute reads as follows:

A directory into which the files should be copied, relative to the root of the Cordova project.

(https://cordova.apache.org/docs/en/8.x/plugin_ref/spec.html#source-file)

Note that cordova-android has diverged from the above documented behaviour in 7.x by replacing parts of specified path for backward compatibility. In fact, the undocumented feature described in this issue is broken (or more like left behind) in that process.

As @brodybits says discussion should be continued on apache/cordova#47.

@breautek
Copy link
Contributor

Closing as obsolete / out-dated.

I don't think this was ever acted on and several people expressed an opposition of even supporting directories, so I think it's probably best to keep the current implementation as-is in modern cordova-android.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants