[#47794] - [6.2] Fix: Add method to copy Codemirror license file to builder
- Pending
- Medium
- Build: 6.2-dev
- # 47794
- Diff
- LadySolveig:6.2/fix/build-licence-codemirror
User tests: Successful: Unsuccessful:
Pull Request resolves #46879 (comment) .
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
Summary of Changes
Add method to copy the CodeMirror license file to the builder. This ensures that the required license file is included in the build output if specified in the build settings.
Build process improvements:
- Added a
copymethod toCodemirrorModuleBuilderthat copies the CodeMirror license file to the target location if specified in the build settings (settings.json). This method first calls the parentcopymethod, then checks for the license filename in the build settings, resolves its path, and copies it to the build output directory.
Testing Instructions
run npm ci
Issues are currently occurring on Windows - for testing this PR with Windows you have to wait that this PR #47793 is merged before, or apply the patch manually.
To validate the test results, please specify the operating system on which the tests were executed.
Actual result BEFORE applying this Pull Request
No LICENCE file in `media/plg_editors_codemirror
Expected result AFTER applying this Pull Request
media/plg_editors_codemirror/LICENSE is availbale in the media folder after build
Link to documentations
Please select:
-
Documentation link for guide.joomla.org:
-
No documentation changes for guide.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript |
| Title |
|
||||||
I have tested this item ✅ successfully on 732952d
I've successfully verified that the media/plg_editors_codemirror/LICENSE file is present after a build with this PR, which is not the case without this PR, and that there are no unexpected other differences compared to a build with the current 6.2-dev branch.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47794.
I have tested this item ✅ successfully on 732952d
I've successfully verified that the media/plg_editors_codemirror/LICENSE file is present after a build with this PR, which is not the case without this PR, and that there are no unexpected other differences compared to a build with the current 6.2-dev branch.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47794.
I have tested this item ✅ successfully on 732952d
I've successfully verified that the media/plg_editors_codemirror/LICENSE file is present after a build with this PR, which is not the case without this PR, and that there are no unexpected other differences compared to a build with the current 6.2-dev branch. I've tested on Linux.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47794.
The failing system tests are not related to this PR.
why do we need a specific method for inclufing the licence for codemirror but not for all other packages? Surely this is a bandage and not a fix to why the code that works for all other packages doesnt work for codemirror. Or is there something specific about codemirror causing this break? Its important to understand the reason so that when we introduce a new package from a new vendor in the future we know if we need to create a specific code for the licence or not
why do we need a specific method for inclufing the licence for codemirror but not for all other packages? Surely this is a bandage and not a fix to why the code that works for all other packages doesnt work for codemirror. Or is there something specific about codemirror causing this break? Its important to understand the reason so that when we introduce a new package from a new vendor in the future we know if we need to create a specific code for the licence or not
@brianteeman The reason why codemirror is special here is because in our composer.json we do not have one dependency for codemirror but several, and there is no master dependency above it, so we have several source folders in node_modules but onmly one target folder in media/vendor. That's why it needs that special treatment.
why do we need a specific method for inclufing the licence for codemirror but not for all other packages? Surely this is a bandage and not a fix to why the code that works for all other packages doesnt work for codemirror. Or is there something specific about codemirror causing this break? Its important to understand the reason so that when we introduce a new package from a new vendor in the future we know if we need to create a specific code for the licence or not
@brianteeman The reason why codemirror is special here is because in our package.json we do not have one dependency for codemirror but several, and there is no master dependency above it, so we have several source folders in node_modules but onmly one target folder in media/vendor. That's why it needs that special treatment.
why do we need a specific method for inclufing the licence for codemirror but not for all other packages? Surely this is a bandage and not a fix to why the code that works for all other packages doesnt work for codemirror. Or is there something specific about codemirror causing this break? Its important to understand the reason so that when we introduce a new package from a new vendor in the future we know if we need to create a specific code for the licence or not
@brianteeman The reason why codemirror is special here is because in our package.json we do not have one dependency for codemirror but several, and there is no master dependency above it, so we have several source folders in node_modules but onmly one target folder in media/vendor. That's why it needs that special treatment.
See https://github.com/joomla/joomla-cms/blob/6.2-dev/package.json#L37-L51 .
why do we need a specific method for inclufing the licence for codemirror but not for all other packages? Surely this is a bandage and not a fix to why the code that works for all other packages doesnt work for codemirror. Or is there something specific about codemirror causing this break? Its important to understand the reason so that when we introduce a new package from a new vendor in the future we know if we need to create a specific code for the licence or not
@brianteeman The reason why codemirror is special here is because in our package.json we do not have one dependency for codemirror but several, and there is no master dependency above it, so we have several source folders in node_modules but onmly one target folder in media/vendor. That's why it needs that special treatment.
See https://github.com/joomla/joomla-cms/blob/6.2-dev/package.json#L37-L51 .
P.S.: The LICENSE file is the same for all of these.
Thank you for the explanation
| Labels |
Added:
PR-6.2-dev
|
||
LGTM
It would be great if you could take another look at this as well @dgrammatiko