Closed (fixed)
Project:
Project Issue File Review
Component:
Miscellaneous
Priority:
Major
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
24 Jan 2011 at 23:30 UTC
Updated:
3 Jan 2014 at 02:58 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
rfayI think this may be one of the most critical items for the git migration.
I'm not sure we're willing to do the migration if it means we lose testing for an unknown period of time. We actually need to discuss that. This might suddenly be the critical path.
Comment #2
boombatower commentedThe key item will be pift since it currently integrates with cvs module...there is the long standing #102102: Parse project .info files: present module list and dependency information which is complete just can't get all the tests to run properly. I would MUCH MUCH prefer it if that were committed (pending final review) and then we ported the entire pift/project stuff to versioncontrol_git. Pifr should be fine...expect the current stable release running on the testbots has a git error...the lastest release does not have this problem...so probably just need to hotfix the clients (otherwise we need to port PIFT to new PIFR api as well).
Comment #3
rfayWe need to set up an entire test environment - and of course that implies that we have to get PIFR/PIFT migrated to see world from the git point of view.
We can use git-dev for the PIFT piece of this, and I would think we could use http://qa.scratch.drupal.org for the PIFR piece. And we can easily come up with testbots.
The big problem is that we're missing working code.
@boombatower: How can we get this moving? This is fundamental. Is #102102: Parse project .info files: present module list and dependency information going to be committable any time soon? If not, we need another approach.
Comment #4
boombatower commentedThe patch is a 100% functional...I just can't get 1 of the 2 testcases to run properly (could just remove it initially).
To outline what we need here:
- d.o clone (for PIFT with git integration)
- qa.d.o clone (to handle git jobs - no code changes...just straight clone)
- pifr worker (with git patch since we cannot run HEAD pifr since qa.d.o has not been updated yet)
Code that needs to be completed:
- pift needs cvs items swapped out with git stuff
-- hook when commit comes in (being added)
-- when building repository checkout string
- I'll roll a patch diffing pifr HEAD (2.3-dev) against 2.2 just for the git plugin (believe 1 line change)
As for the .info patch...it reference a number of cvs things...I would like to see it get in and ported with this...as it will solve a number of other problems people have reported, but obviously it is not necessary.
Comment #5
webchickYeah, this is a release blocker IMO. :( Especially with Drupal 8 opening up around the corner...
Thanks for your attention to this, guys. I guess this fell off my radar because back in the day it looked like it was just going to be a matter of changing 'cvs checkout' to 'git clone'. :(
Comment #6
rfayThanks for the detailed response, @boombatower!
So... how about this plan.
1. Sam will allow access and setup of git-dev.drupal.org for the PIFT part
2. We'll hijack qa.scratch.drupal.org for the PIFR server part
3. We can make testbots as necessary.
So we already almost have what we need.
How can we support this, @boombatower? Would a weekend sprint be in order or something?
Comment #7
boombatower commentedHere is the promised patch to add the git.inc VCS backend currently in HEAD.
@rfay: have a 3pm CST meeting with sdboyer today to hopfully would through PIFT port. Then all the code should be done.
Comment #8
marvil07 commentedYou can use
git log -1 --format="%H"and avoid the preg_match ;-)Powered by Dreditor.
Comment #9
boombatower commented#1041330: Port from cvs module to versioncontrol api
Comment #10
boombatower commentedThe patch will need to be applied to qa.d.o so it allows git as a repository type. So this will need to be done in staging environment.
Comment #11
eliza411 commentedComment #12
nnewton commentedqa.scratch.drupal.org setup now, waiting on workers.
Comment #13
rfayI had to start httpd on scratchvm, but it's working fine.
There is now a PIFR testbot configured for it: http://ec2-67-202-0-173.compute-1.amazonaws.com. Normal username and password per http://qa.drupal.org/node/84 #16.
Applied the patch http://drupal.org/files/issues/1038830-git.inc_.patch
boombatower is going to help me figure out why it's using cvs still instead of git.
Comment #14
rfayA starter list of things that had to be set up:
1. Install git on testbot. We need 1.7, so have to get this somewhere not from the regular debian repo.
apt-get install -t lenny-backports git-core.2. Apply the patch http://drupal.org/files/issues/1038830-git.inc_.patch in /var/lib/drupaltestbot/sites/all/modules/project_issue_file_review.
3. Change the client config on qa (and the host it points to)
4. Change the config on git-dev (and the host it points to)
Comment #15
webchickComment #16
boombatower commentedSince we aren't running latest code we need a few additional changes.
server (qa.d.o)
- #7 http://drupal.org/files/issues/1038830-git.inc_.patch
- this patch
workers
- just #7
Comment #17
boombatower commentedAdded --depth 1 to git clone command.
Rolled full patch against that can be applied to server and workers (even though only half the changes are needed for each...keeps it simple). It required a bit of reworking since we aren't running latest code which would require less hassle.
Comment #18
boombatower commentedRfay and I decided to use 7.0 for client forfirmation.
Comment #19
boombatower commentedMissed debug line.
Comment #20
rfayDuring confirmation, on the syntax patch, it's attempting to patch /var/lib/drupaltestbot instead of /var/lib/drupaltestbot/checkout. What's up with that? As a result, it gets a permission denied trying to save the .orig file. And, of course, it would not be able to check the syntax error.
Edit: see http://qa.scratch.drupal.org/pifr/test/113124
Comment #21
boombatower commentedAs far as I know there is no change relevant to that...not really sure what to say. I just reviewed the code again and the apply is the same for cvs and git.
Comment #22
boombatower commentedPrevious patches were malformed.
Comment #23
rfayHere's a patch with the git.inc - we confirmed one with this.
Comment #24
rfayThis is #23 with
* the patch files removed
* the change to use git-dev.drupal.org as the $core_url (because it doesn't have $Id$ in it)
* And a change to report the directory in which a failed command was executed.
In fact, when it tries to test Examples, it
* Checks it out into checkout/sites/default/modules (not checkout/sites/default/modules/examples)
* Tries to apply the patch in /var/lib/drupaltestbot/ (completely wrong)
The most recent run was http://qa.scratch.drupal.org/pifr/test/126194. It shows the enhanced directory information in the #fail.
Comment #25
rfayOoops. the patch.
Comment #26
rfayCurrently unable to confirm clients with them set on "all tests". http://qa.scratch.drupal.org/pifr/test/113124 and the identical http://qa.scratch.drupal.org/pifr/test/126174 (both testbots reacted the same).
Again, what happens is that the patch attempts application in the wrong directory. The new information in the review log (#24/25)explains that.
[14:32:10] Command [patch -p0 -i /var/lib/drupaltestbot/sites/default/files/review/syntax.patch 2>&1] failed in directory [/var/lib/drupaltestbot] with status [2] and output:
patching file install.php
Hunk #1 FAILED at 5.
patch: **** Can't rename file install.php to install.php.orig : Permission denied.
[14:32:10] Encountered error on [apply], details:
array (
'@filename' => 'syntax.patch',
)
Comment #27
rfayFor convenience of debugging and deployment, I've forked this and it's at https://github.com/rfay/project_issue_file_review. Boombatower and sprice have commit. I'm using that version on the two testbots.
Comment #28
rfayMade some progress tonight. The field names in the database setup page have changed: (mysql[password], etc.). Please see the github repo. I'm going to try to do a full client reconfirm and go to sleep!
Comment #29
rfayI manually changed the URLs on qa.scratch in the pifr code to git-dev.drupal.org. That's a manual change that has to make it into the code (which it is in the client code... I don't understand why we're patching 2 different places. We should get both patches into the same codebase... github)
Comment #30
boombatower commentedDid we start from the patched code the client where already running? As there are a few patches necessary for pifr 2.2 to work since core installer changed and what not.
Comment #31
rfayHmm... No, I started from 2.2. Thought you had said that was golden. Lose track. Well. we'll get this stuff all organized. Can you point me to the information about the patch for the PIFR testbot?
Comment #32
rfayDon't worry about it - I compared the base code to the deployed code, and there was only one real change, and it was to cover the issues in #28, which are already covered here.
Comment #33
rfayI created 6.x-3.x-dev (DRUPAL-6--3).
Here is the first patch applied to it, which successfully tests D7 core and a D7 patch.
Yet to be tested:
Committed to DRUPAL-6--3: http://drupal.org/cvs?commit=497748
Comment #35
rfayI'm calling this fixed. We have an environment. There are issues to be discovered, but everything seems to be right now.
All the code is now in CVS in DRUPAL-6--3 on both PIFR and PIFT.
I will be using CVS for commits from here on in and not using the github repo.
Comment #36
webchickYEAH!!!!!!!!!! :D Great work, guys!!!
Comment #37
boombatower commentedJust want to make sure #1057146: Migration-day script to flush all branch test results and queue them up again is linked here since a lot of folks on this issue will probably want to see that fixed.
Great to see all those who helped on this issue.