Closed (won't fix)
Project:
Drupal.org drush
Version:
7.x-1.x-dev
Component:
Miscellaneous
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
29 Nov 2012 at 23:29 UTC
Updated:
20 Jun 2016 at 23:34 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
geerlingguy commentedI don't know if we've discussed this specific issue in either #1470692: Add Grammar Parser to white list (or provide an alternative solution) or somewhere in #1701784: Please whitelist Aloha Editor, but I thought the consensus was that another route could be used to include certain things from internal projects...? That said, I haven't really had to deal with this, and don't know the reasoning behind allowing or not allowing this exception, so I'd rather defer to a different whitelist maintainer or an infrastructure person.
Comment #2
geerlingguy commentedMarked #1940330: Add drupalcode.org into packaging whitelist as a duplicate.
Comment #3
PatchRanger commentedHere is another reason to whitelist drupalcode.org:
I need drupalcode.org to be added to whitelist - is it the same as http://git.drupal.org (or how is it related to it), which is already whitelisted? I am using it as a source of Drupal-major-version-independent release of my module (see http://drupal.org/project/botcha#installation for an example). This release is used as a library by Drupal-major-version-dependent releases (aka adapters), which are usual Drupal releases. See #1932290: Forwardport doctrine - a proposed solution for Drupal major version migration problem for more info about this way of code management.
Comment #4
btopro commentedI don't believe you need this white-listed in order to use git specific versions. The drush documentation on this should work unless it's specifically blocked. Another alternative that i've used in the past to use profiler module is this
Which has successfully gotten past the d.o. packaging white-list. Is there a problem w/ using the ftp based address for this?
Comment #5
btopro commentedHere are some additional examples from the https://drupal.org/project/drupalapi distro
Hope this helps
Comment #6
kreynen commentedClosing this as "works as designed". Full, promoted projects are already available using git. While I advocate #1971774: Standardize whitelisted regex and update license tags, those are per whitelist entries. Adding a wildcard for everything on drupalcode.org would include sandboxes. Code from sandboxes has never been allowed. This is already stated on distribution packaging requirements
To quote from @dww in #1432326: Support for git clones from Drupal.org sandboxes
As far as...
Write a patch to remove the files. Patches can delete as well as add and modify. See #1337004: Remove CKEditor demos directory
Comment #7
dave reidI'm going to re-open this, sorry. With the issue queue redesign, as a maintainer I'm finding myself committing things directly without uploading patches because it takes *way* longer than it did before. And I've also encountered several different situations where the patch I provided to the maintainer was tweaked before commit, and causes other patches applied to the project to break.
I respectfully think we should be able to whitelist the following pattern, which would limit it to patches generated by drupalcode only (and would prevent sandboxes from being allowed):
Comment #8
btopro commented@kreynen this looks reasonable to me as well as the rationale though I have a question. Marking that we need further review but as a distro maintainer I see how you could have a potential conflict w/ a developer downstream of how a patch should function.
I'm still not entirely sure though why you couldn't upload the patch and point to it in the queue via the normal patch array in drush make file, and then point to a specific git checkout of the module/theme as a whole to apply the patch to. Dave Reid is there a reason that wouldn't work?
Comment #9
kreynen commentedUnfortunately the URL path to patches has already changed since @David Reid re-opened the issues. A sandbox (I think we all agree should be blocked?) looks like...
http://cgit.drupalcode.org/sandbox-websolutions.hr-2262991/patch/?id=25c...
A promoted project looks like...
http://cgit.drupalcode.org/xmlsitemap/patch/?id=ab72f26d21c8ad53633bd571...
So at best, the regex needs to be updated.
I still don't understand the request since you can already include a specific git commit of a promoted project in the .make. The only the use case I can come up with is someone with commit access to the module project is committing to an unreleased branch and a distribution wants to commit those changes as a patch to the packaged release of the module so the module has a version number, but I don't understand why they wouldn't just include the unreleased branch using git in the .make.
Can either of you provide a link to a specific commit they wanted to use as a patch in a distribution?
Comment #10
dave reidI've quite commonly seen modules that have unrelated or major changes added in a dev release before the patch I had provided was committed, so want the last stable release + the patch I provided, which the maintainer tweaked before committing. I don't want to use a specific git commit because that means I am forced to also use all the commits since the version I want was created.
Comment #11
kreynen commentedThanks. The use case for this is making more sense. You want to include a change that is already in git and is (or will be) packaged into the dev snapshot without all the other changes that may also be committed by generating a patch from a specific commit using http://cgit.drupalcode.org.
The big advantage/timesaving is that you wouldn't need to save the output from http://cgit.drupalcode.org, go back to the issue and add an updated patch to the issue that the maintainer likely closed when they made the commit. You'd have the change with any modifications the maintainer made before committing it as it will likely be in the next release without any other potentially unfinished/problematic changes.
As long as the regex requires patch in the path and doesn't allow distributions to include entire sandbox modules, I actually don't care if the patch is actually from a fork of a fully promoted project. While it lacks features of upstream synchronizations that GitHub has, forking a project into a sandbox is fairly common on Drupal.org and is actually an easy way to generate and discuss complicated patches.
So the updated regex would look something like...
^http://cgit.drupalcode.org/.+\/patch/?id=.+$
Comment #12
btopro commentedconcept makes sense to me, just wanted it elaborated a bit. I'm ok with the regex as suggested since this would just cover patches and prevent whole sandboxes from inclusion which we intentionally avoid. All other use-cases should then use git specific commits.
Comment #13
geerlingguy commentedSo, put inside code tags and cleaned up:
^http://cgit\.drupalcode\.org/.+\/patch/\?id=.+$According to regexpal, this would match things like:
and not:
So I think we're good here. If someone gives one more eyeball/approval and sets to RTBC, one of the maintainers will add the regex and finally get this 2.5-year-old issue (one of the originals :) closed!
Comment #14
kreynen commentedI went ahead and added https://www.drupal.org/node/2291151. While this addresses @Dave Reid's request to be able to apply specific commits as a patch, it does NOT address @Deciphered's original request which was to whitelist everything in the drupalcode.org domain so he could cherry pick specific files from the Documentation project. We've already addresses the possible solutions for that as well as the downside of allowing sandbox projects in distributions.
So this is "fixed" for patches and "will not fix" for the rest of drupalcode.org.
Comment #16
torotil commentedAlthough the whitelist entry is present it still doesn't work for patches. The regular expression for valid patch-urls is hardcoded in drupalorg_drush:
Should we reopen this issue? Should I open a separate one for drupalorg_drush?
Comment #17
kreynen commentedMoved to Drupal.org Drush project so we don't have to rehash the reason why someone would want to use this pattern.
I think the right solution would be to add ^https?://(www\.)?drupal.org/files# to the whitelist and stop hardcoding regular expressions into the drupalorg_drush project, I'll let @drumm decide which fix (if any) he wants to implement.
Comment #18
mrharolda commentedAny progress on this one?
Comment #19
mkhamash commentedI have added the cgit.drupalcode.org to allowed project patches URLs patterns, though it is still a hardcoding for this pattern and not using the whitelist, since the intention of whitelisting cgit.drupalcode.org should be to allow patches to drupal hosted full projects from this URLs patterns only as a special case, and not allow patches to projects from all the whiteliste pattern list, also the code for verifying drupal project patches is a different from verifying libraries URLs.
If the intention was to simplify the code for verifying URLs and to manage all URL patterns from the whitelist for projects and libraries, I would be more than happy to re-roll another patch for this.
Comment #20
drummDrupalcode.org shouldn't be depended on for patches, or anything much more than browsing.
Drupalcode.org was previously changed from GitWeb to cgit for serving files. This changed all the URLs and we didn't manage to get all URLs redirecting. In the long run, I expect this to happen again.