Problem/Motivation
After working on #2281429: [js] D8 Port for lengthy amount of time today, I finally got to a good enough stopping point where I felt it was safe to finally "enable" the module. Much to my surprise: the module couldn't be "found".
I spent a ton of time trying to figure out if it was a file permission issue, symlink issue, parse issue... only to finally remember that there was a "blacklist" somewhere that may be causing me grief.
Sure enough, this valid D7 module cannot be ported using the same name because the entire directory is skipped. This is a major problem considering the namespace (on d.o) is already being used.
Yes, I read the doc comment:
/**
* List of directory names to skip when recursing.
*
* These directories are globally ignored in the recursive filesystem scan;
* i.e., extensions (of all types) are not able to use any of these names,
* because their directory names will be skipped.
*
* @var array
*/
However, that is a really... really stupid "reason".
Especially considering the "fix" is really simple: just check if there's an immediate .info.yml file directly inside it.
Before anyone tells me to "just rename the module", that isn't an option. I don't want to have to try and come up with a new "name" when all the code (in 7.x) and the stuff I just did in 8.x centers around "js". It'll make backporting a PITA.
Proposed resolution
Loosen blacklist to allow modules that have an .info.yml directly inside the folder that's blacklisted.
Remaining tasks
- Create a patch
- Create tests
User interface changes
None
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2839191-2.patch | 2 KB | markhalliwell |
Comments
Comment #2
markhalliwellComment #5
markhalliwellEscalating to critical, if only to gain attention and feedback after half a year...
Please feel free to deescalate.
Comment #6
mile23If this is a critical, then it's a critical in 8.3.x, unless you want to wait a year before anyone can install the js module... I think this issue might be critical in that it disallows contrib development for existing projects without a reasonable workaround. Adding the 'needs critical triage' tag to get another opinion.
So we avoid js directories for speed reasons, which is reasonable. But it's also a problem if an extension name is in the blacklist. So therefore checking whether it's an extension is also reasonable. It adds a single file_exists() per blacklisted directory name, but this is still better than a full recursive scan of the entire directory.
The patch still applies to 8.3.x, and allows the js module to be discovered.
Comment #7
catchThe patch fails, so it can't be RTBC, it could also use some test coverage (i.e. a test module matching a blacklist directory and see if it can be found).
This isn't critical, since renaming the module is an obvious workaround (if not a pleasant one), but checking for .info.yml in the blacklisted directory might be OK. Main question I have is whether this means the js module is then prevented from ever having a submodule, or does the .info.yml presence allow the scan to go a further directory level down?
Comment #11
andypostComment #13
markhalliwellI'm just going to close this. It's been years since I've opened it and a D8 port of this module is likely never to happen since D8 has rest and jsonapi now.