Hi Daniel,
Thanks for the patches; I appreciate that.
I'd like to make a few comments though.
The first patch, to DownloadQueue.hs, doesn't actually make a difference
to the program execution. getProcessStatus only returns Nothing if it
was run in non-blocking mode and the given pid hadn't exited yet (or it
was given an invalid pid). As it was in the version you patched, that
would never occur. (I have since modified that code to not even deal
with Maybes in the first place). The proper thing to do for curl errors
is to examine the status and see if it was something other than
ExitSuccess.
For the posthook, I think that is a great idea. I wonder, though, if it
would be better to let a person define the hook in the config file
rather than in the database. This would make it easy to set a
post-process hook for all podcasts (they could put it in the [DEFAULT]
section).
What do you think?
On Sun, Mar 11, 2007 at 09:33:40PM -0700, Daniel Burrows wrote:
> In some circumstances, it's useful or necessary to postprocess
> podcasts after downloading them. For instance, my little music player
> can play videos too, but only if they're in a very particular format. A
> posthook could be used to
>
> The attached patch implements this by adding a "setposthook" command,
> which takes the full pathname of a script to execute after downloading a
> podcast. The script should take a single argument, the full pathname of
> the podcast.
>
> Daniel
>
> -- System Information:
> Debian Release: 4.0
> APT prefers unstable
> APT policy: (500, 'unstable')
> Architecture: i386 (i686)
> Shell: /bin/sh linked to /bin/bash
> Kernel: Linux 2.6.18-4-686
> Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
>
> Versions of packages hpodder depends on:
> ii curl 7.15.5-1 Get a file from an HTTP, HTTPS,
> FT
> ii id3v2 0.1.11-3 A command line id3v2 tag editor
> ii libc6 2.3.6.ds1-13 GNU C Library: Shared libraries
> ii libgmp3c2 2:4.2.1+dfsg-4 Multiprecision arithmetic library
> ii libsqlite3-0 3.3.8-1.1 SQLite 3 shared library
>
> hpodder recommends no packages.
>
> -- no debconf information
>
> New patches:
>
> [Add space in the database and in the Podcast data structure for a
> possibly-missing "posthook" field.
> Daniel Burrows <[EMAIL PROTECTED]>**20070312023823] {
> hunk ./Commands/Add.hs 37
> + postHook = Nothing,
> hunk ./Commands/ImportIpodder.hs 100
> + postHook = Nothing,
> hunk ./DB.hs 71
> -upgradeSchema dbh 3 _ = return ()
> +upgradeSchema dbh 4 _ = return ()
> +
> +upgradeSchema dbh 3 tables =
> + do dbdebug "Upgrading schema 3 -> 4"
> + dbdebug "Adding posthook column"
> + run dbh "ALTER TABLE podcasts ADD posthook STRING NULL DEFAULT NULL"
> []
> + setSchemaVer dbh 4
> + commit dbh
> + upgradeSchema dbh 4 tables
> hunk ./DB.hs 159
> - run dbh "UPDATE podcasts SET castname = ?, feedurl = ?, pcenabled = ?, \
> + run dbh "UPDATE podcasts SET castname = ?, feedurl = ?, posthook = ?,
> pcenabled = ?, \
> hunk ./DB.hs 162
> + toSql (postHook podcast),
> hunk ./DB.hs 175
> - do res <- quickQuery dbh "SELECT castid, castname, feedurl, pcenabled,
> lastupdate FROM podcasts ORDER BY castid" []
> + do res <- quickQuery dbh "SELECT castid, castname, feedurl, posthook,
> pcenabled, lastupdate FROM podcasts ORDER BY castid" []
> hunk ./DB.hs 180
> - do res <- quickQuery dbh "SELECT castid, castname, feedurl, pcenabled,
> lastupdate FROM podcasts WHERE castid = ? ORDER BY castid" [toSql wantedid]
> + do res <- quickQuery dbh "SELECT castid, castname, feedurl, posthook,
> pcenabled, lastupdate FROM podcasts WHERE castid = ? ORDER BY castid" [toSql
> wantedid]
> hunk ./DB.hs 197
> -podcast_convrow [svid, svname, svurl, isenabled, lupdate] =
> +podcast_convrow [svid, svname, svurl, svposthook, isenabled, lupdate] =
> hunk ./DB.hs 199
> + postHook = fromSql svposthook,
> hunk ./Types.hs 47
> + postHook :: Maybe String,
> }
>
> [Run the podcast posthook, if any, in the "I finished downloading" callback.
> Daniel Burrows <[EMAIL PROTECTED]>**20070312042724] {
> hunk ./Commands/Download.hs 22
> +import Data.Foldable(for_)
> hunk ./Commands/Download.hs 38
> +import System.Posix.IO(
> + OpenMode(..),
> + closeFd,
> + defaultFileFlags,
> + dupTo,
> + openFd,
> + stdOutput
> + )
> hunk ./Commands/Download.hs 165
> + let pcPostHook = postHook $ podcast ep
> + for_ pcPostHook (runPostHook finalfn)
> hunk ./Commands/Download.hs 179
> +-- | Given a podcast's post-hook and a downloaded episode of that podcast,
> run the
> +-- podcast's post-hook on the episode.
> +--
> +-- stdout is redirected to /dev/null.
> +runPostHook :: String -- | The file-name of the episode to process.
> + -> String -- | The post-hook to execute.
> + -> IO ()
> +runPostHook epFileName postHookCmd =
> + do kid <- forkProcess startPostHook
> + (Just status) <- getProcessStatus True False kid
> + case status of
> + Exited ExitSuccess -> return ()
> + Exited (ExitFailure n) -> error $ "Post-hook " ++ postHookCmd ++ "
> exited with status " ++ (show n) ++ "\n"
> + Terminated sig -> error $ "Post-hook " ++ postHookCmd ++ " was
> killed by signal " ++ (show sig) ++ "\n"
> + Stopped _ -> error "The world is coming to an end: I didn't ask to
> be woken when my child stopped, yet I was!"
> + where startPostHook =
> + do bracket (openFd "/dev/null" ReadOnly
> + Nothing defaultFileFlags)
> + closeFd
> + (\devNull ->
> + do dupTo devNull stdOutput)
> + executeFile postHookCmd False [epFileName] Nothing
> +
> }
>
> [Add a command "setposthook" that sets the posthook of a podcast.
> Daniel Burrows <[EMAIL PROTECTED]>**20070312042748] {
> hunk ./Commands.hs 49
> +import qualified Commands.SetPostHook
> hunk ./Commands.hs 68
> + Commands.SetPostHook.cmd,
> addfile ./Commands/SetPostHook.hs
> hunk ./Commands/SetPostHook.hs 1
> +{- hpodder component
> +Copyright (C) 2006 John Goerzen <[EMAIL PROTECTED]>
> +
> +This program is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 2 of the License, or
> +(at your option) any later version.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program; if not, write to the Free Software
> +Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +-}
> +
> +module Commands.SetPostHook(cmd, cmd_worker) where
> +import Utils
> +import System.Log.Logger
> +import DB
> +import Download
> +import FeedParser
> +import Types
> +import Text.Printf
> +import Config
> +import Database.HDBC
> +import Control.Monad
> +import Utils
> +import Data.String
> +import System.IO
> +import System.Console.GetOpt
> +import System.Console.GetOpt.Utils
> +import Control.Exception
> +
> +i = infoM "setposthook"
> +w = warningM "setposthook"
> +d = debugM "setposthook"
> +
> +cmd :: (String, Command)
> +cmd = simpleCmd "setposthook"
> + "Alter the posthook of a podcast" helptext
> + [Option "c" ["castid"] (ReqArg (stdRequired "castid") "ID")
> + "Podcast ID to modify",
> + Option "p" ["posthook"] (ReqArg (stdRequired "posthook") "CMD")
> + "Command to run after downloading episodes of this podcast",
> + Option "d" ["delete"] (NoArg ("delete", ""))
> + "Remove any currently set post-hook for this podcast"]
> + cmd_worker
> +
> +cmd_worker gi (args, []) =
> + do podcastid <- case lookup "castid" args of
> + Just x -> return (read x)
> + Nothing -> fail "setposthook: --castid required; see
> hpodder setposthook --help"
> + newPostHook <- case (lookup "posthook" args, lookup "delete" args) of
> + (Just _, Just _) -> fail "You may not pass both
> --delete and --posthook to setposthook"
> + (Nothing, Nothing) -> fail "You must pass exactly
> one of --delete and --posthook to setposthook"
> + (Just val, Nothing) -> return $ Just val
> + (Nothing, Just _) -> return $ Nothing
> + pc <- getPodcast (gdbh gi) podcastid
> + case pc of
> + [x] -> updatePodcast (gdbh gi) (x {postHook = newPostHook})
> + _ -> fail $ "Invalid podcast ID given"
> + commit (gdbh gi)
> +
> +cmd_worker gi (_, _) =
> + fail $ "setposthook: unrecognized arguments; see hpodder setposthook
> --help"
> +
> +
> +helptext = "Usage: hpodder setposthook -c CASTID (-p 'CMD' | -d)\n\n\
> + \You must specify one podcast ID with -c, and either specify the\n\
> + \new posthook with -p, or provide -d to remove a previously-set posthook.\n\
> + \The posthook should be an absolute pathname; it will be passed the\n\
> + \file name of a newly downloaded episode as its one and only argument.\n"
> }
>
> Context:
>
> [Readying for hackage
> John Goerzen <[EMAIL PROTECTED]>**20070307004153]
> [TAG RELEASE_hpodder_0.99.1
> John Goerzen <[EMAIL PROTECTED]>**20061208111652]
> Patch bundle hash:
> 70483b8db5b15e1e03f553599657f579a263b445
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
