#3307163: Add the ability to ProjectInfo handle projects not in the active codebase follow-up
right now it is up to any module that uses package_manager to check drupal.org XML to check that any releases are secure, published and supported and that project itself is supported
That means there will be a lot of duplication and package_manager itself will install insecure releases.
After #3307163: Add the ability to ProjectInfo handle projects not in the active codebase is done all new or updated packages that start with `drupal/` and are projects that would have Update XML should be checked with Update XML.
\Drupal\package_manager\Validator\ReleaseValidator this would similar to \Drupal\automatic_updates_extensions\Validator\UpdateReleaseValidator except it should only subscribe to PreApply and it should check updated as well as new packages\Drupal\automatic_updates_extensions\Validator\UpdateReleaseValidator. This should be covered by the new validatorThis is mean this validation would apply to all modules that use Package Manager.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
tedbowComment #3
tedbowComment #4
tedbowComment #5
narendrarAdded tag for Install directly via GUI using Package Manager + core APIs
Comment #7
tedbow#3303902: Move ProjectInfo class into package_manager is done!
Comment #8
tedbowComment #10
kunal.sachdev commentedComment #11
tedbowChanging to Needs work to indicate there is any active merge request
"Active" means work has not started on the code change
Comment #12
kunal.sachdev commentedCurrent test failure is going to be handled in #3305564: Create a validator to stop newly installed packages from overwriting existing directories on apply
Comment #13
kunal.sachdev commentedComment #14
phenaproximaIn that case, let's postpone this issue on that one.
Comment #15
phenaproximaI just committed the blocker.
Comment #16
kunal.sachdev commentedComment #17
phenaproximaI think this makes sense.
My main concerns are about fixture drift, where we add a lot of fixtures that aren't very well documented or clear as to their purpose. I want to correct existing problems with this in #3309486: [meta] Make all fixtures consistent, but I also don't want to introduce more issues here. :)
It's also not going to fly for Package Manager tests to extend Automatic Updates' test classes; instead, we should move whatever methods we need from AU into Package Manager.
My final overarching issue is that this seems to introduce a pervasive need to disable the supported releases validator in several tests. Disabling validators is something we should always seek to avoid, unless there's no other option. I think we might still be able to get around the validator by making most of our fixtures' stub modules of the
drupal-custom-moduletype.Comment #18
phenaproximaBeen thinking about this a lot and I think that the right thing to do is proceed with disabling the validator in the tests that need to disable it, at least for now, but over in #3309486: [meta] Make all fixtures consistent, we should change our fixtures so they only use test modules that we have fake release data for.
That means we'll probably need to create a bunch of reusable stub modules (and at least one stub theme), and a bunch of release XML for each one.
This is really only a problem since Package Manager became aware of project history. It no longer makes sense to have fixtures that refer to projects with no history.
Comment #19
kunal.sachdev commentedComment #20
phenaproxima@kunal.sachdev, it turns out the problem you're running into (I did as well) is due to #3308686: Logic error in XdebugValidator. I reviewed and committed that, and have merged it back into this branch, so I think you'll be all set now.
Comment #21
kunal.sachdev commentedComment #22
phenaproximaOne nit, then I think I'm happy with the way this looks.
Comment #23
kunal.sachdev commentedComment #24
phenaproximaNo further objections here. We'll clean up the fixtures and re-enable the disabled validator in subsequent issues, since that depends on us having coherent release history for all stub modules known to Drupal.
Awesome work, @kunal.sachdev. Thanks for sticking with it so persistently.
Comment #25
drummComment #26
tedbowcouples questions on this
If I am correct about the missing test coverage this could be added. the duplicate fixtures could be solved in a follow-up
Comment #27
kunal.sachdev commentedComment #28
tedbowLooks good!
Comment #30
tedbow🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉
@kunal.sachdev and @phenaproxima thanks! 🎉 🎉 🎉
Huge win security and package_manager 🎉 🎉 🎉 🎉
🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉
Comment #32
tedbow