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 |
| #2 | package-release-nodes.php.1.txt | 10.22 KB | dww |
| package-release-nodes.php.0.txt | 5.93 KB | dww |
Comments
| Status | File | Size |
|---|---|---|
| new | package-release-nodes.php.1.txt | 10.22 KB |
...
| Status | File | Size |
|---|---|---|
| new | package-release-nodes.php.2.txt | 11.55 KB |
new version that does all the msg* mojo to correctly package translations.
translation status stuff is still being postponed for now.
this needs work before it can go live:
- never call `rm` with a relative path
- rip out the debugging code that was left in the previous version. ;)
- fix $exclude vs. $to_exclude confusion
just 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.
| Status | File | Size |
|---|---|---|
| new | package-release-nodes.php.3.txt | 13.62 KB |
- 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)...
the 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,
natrak 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...
| Status | File | Size |
|---|---|---|
| new | package_release_nodes_code_style.patch.txt | 7.4 KB |
doesn'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 and print() 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.
| Status | File | Size |
|---|---|---|
| new | package-release-nodes.inc.txt | 2.51 KB |
committed 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 global everywhere to get this working)...
so, the only remaining TODO items from comment #8 are:
- cvs checkout vs. cvs export (good idea, should fix)
- chdir() failing when cvs checkout fails -- needs better error checking (good idea, should fix)
- "msgattrib: internationalized messages should not contain the `\r' escape sequence" (no idea about this)
therefore, i'm moving this back to "needs work" for now...
| Status | File | Size |
|---|---|---|
| new | package_release_nodes_error_checking.patch.txt | 7.83 KB |
major cleanup of error checking, logging, and robustness. ;)
- use
cvs exporteverywhere instead ofcvs checkoutsince that avoids having to exclude the "CVS" directories all over the place. - added
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... - added
drupal_chdir()which is a wrapper forchdir()that logs errors to the watchdog. - all shell commands now use the appropriate
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 going - all errors are now logged to "release_error" watchdog type (where's this 16 char max coming from, anyway? :( boo, hiss...).
- we attempt to blow away the directory we're about to
cvs 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?
Kjartan in IRC said this was RTBC, so i committed to DRUPAL-4-7--2 branch as revision 1.1.2.11.
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...