Closed (fixed)
Project:
Project
Version:
4.7.x-1.x-dev
Component:
Releases
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
8 Oct 2006 at 11:02 UTC
Updated:
23 Nov 2006 at 10:30 UTC
Jump to comment: Most recent file
here's an initial version of a packaging script for the new release system, in particular, the conversion of releases into real nodes (http://drupal.org/node/83339).
this still doesn't handle periodic rebuilding from branches, nor translation reporting stuff, but it works just great for releases from tags, including updating the release node with the file, md5 hash, and date, and then publishing the node.
it's all CLI-drupal for now, though potentially we could split some of this up into a .inc file that was called directly from project_release.module when new tag-based releases are added...
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | package_release_nodes_error_checking.patch.txt | 7.83 KB | dww |
| #12 | package-release-nodes.inc.txt | 2.51 KB | dww |
| #10 | package_release_nodes_code_style.patch.txt | 7.4 KB | dww |
| #6 | package-release-nodes.php.3.txt | 13.62 KB | dww |
| #3 | package-release-nodes.php.2.txt | 11.55 KB | dww |
Comments
Comment #1
dwwnew version:
TODO:
i agreed with killes that the best way to handle translation stats is a) store that info in the DB, not a flat file, and b) add a simple little translation_stats.module that displays the info from the DB into a human-readable table at some fixed menu location. however, it's not worth delaying the entire release system for this added complication, so translation stats will probably happen in a separate version of all this stuff, once the main system is live. probably on the order of a week after the initial release...
Comment #2
dww...
Comment #3
dwwnew version that does all the msg* mojo to correctly package translations.
translation status stuff is still being postponed for now.
Comment #4
dwwthis needs work before it can go live:
Comment #5
dwwjust committed everything from this issue into the DRUPAL-4-7--2 branch of project/release (that seemed to make the most sense as the place to put this script, even though it depends on cvs.module, too).
also, i fixed #1 and #2 from the previous comment. so, the full directory path we're sending to rm -rf and better logging/error propagation is all that remains to be done.
Comment #6
dww- cleaned up all the customization variables
- all external tools have a setting for the full path so we never have to rely on PATH
- never invoke rm with relative path to the directory to destroy
- removed debugging code that was left in
- other fixes
RTBC?
(the only thing i could see as a minor safety measure is to validate that the $tmp_dir you specify has to be an absolute path)...
Comment #7
dwwthe version from comment #6 is certainly a vast improvement over what was in CVS already, so i went ahead and committed it to the DRUPAL-4-7--2 branch as revision 1.1.2.5. kjartan installed it on scratch.d.o and it seems to be working fine.
however, i'd still like to get a thorough review from kjartan or killes before i mark this as fixed/closed.
Comment #8
Kjartan commentedJust some quick notes:
- Can we move the config to an include file? The settings we need on the d.o infrastructure are different than the default, including a lot of the external tool paths. Having it all in one file makes it easy to miss the path to tar being wrong when updating.
- Any reason to use cvs checkout? wouldn't export be better as we then can avoid excluding the CVS directories in all other commands.
- The chdir() on line 250 can fail when cvs co has nothing to checkout, would be a good point to include a little check as it can be rather hard to track down real errors in the output with a ton of minor warnings showing up. We probably have some junk in the various release tables causing this, so another option is to clear out the stuff that doesn't exist.
- There are a couple of Drupal coding standard issues (incorrect indentation, string concats, inconsistent use of t()), but not going to be too picky about that.
- The translation packaging seems to spew out a ton of "msgattrib: internationalized messages should not contain the `\r' escape sequence", these can probably be ignored safely but figured I would mention it in case someone has any ideas on how to avoid them.
Apart from that it looks good,
Comment #9
dwwnatrak and i agree none of this is critical enough to delay deployment of the new system. but, these are all good suggestions, so i'll work on them time permitting...
Comment #10
dwwdoesn't solve everything, but here's a start... fixes the stray whitespace and indentation issues i could find, along with the inconsistent use of
t()(which also fixes a few *potential* watchdog XSS holes). ;) of course, we can't use t() before we successfully bootstrap ourselves, so the initial error checking andprint()statements have to be untranslatable, but i don't see any problem with that...Comment #11
Kjartan commentedSeems to fix up the worst of the offenses.
Comment #12
dwwcommitted patch from #10 to DRUPAL-4-7--2 as revision 1.1.2.7.
in terms of the config settings in a separate file, Kjartan and i decided in IRC it'll be a lot easier to just customize this locally by just putting all the config variables in a separate .inc file and adding a single line to the live script. this will make it much less likely to have conflicts when updating, etc. attached is an example of such a local config .inc file (notice the use of
globaleverywhere to get this working)...so, the only remaining TODO items from comment #8 are:
therefore, i'm moving this back to "needs work" for now...
Comment #13
dwwmajor cleanup of error checking, logging, and robustness. ;)
cvs exporteverywhere instead ofcvs checkoutsince that avoids having to exclude the "CVS" directories all over the place.drupal_exec()which is a wrapper forexec()that logs any errors (safely) to the watchdog. if the command failed , all output (including stderr) is logged...drupal_chdir()which is a wrapper forchdir()that logs errors to the watchdog.drupal_*wrapper so we log any failures. plus, failures in the middle of packaging a given release (e.g.cvs exportfails) cause that package attempt to abort, so we don't do unsafe things and keep goingcvs exportinto, so that we don't get any errors about things being in the way...i think that's everything i plan to fix here. ;) RTBC?
Comment #14
dwwKjartan in IRC said this was RTBC, so i committed to DRUPAL-4-7--2 branch as revision 1.1.2.11.
Comment #15
(not verified) commented