On 04/09/13 06:31, Dave Reisner wrote:
On Tue, Sep 03, 2013 at 03:40:24PM +1000, Allan McRae wrote:
2) At the end of this patch series we have ended up with this in makepkg:
source $LIBRARY/downloads.sh source $LIBRARY/extractions.sh source $LIBRARY/utils.sh
I think it would be best to have a file $LIBRARY/source.sh that is sourced in makepkg and its role it to source all the components of the library.
I'm not sure how much value there is in doing this. If the idea is to have a "toplevel" file for each directory of sublibs, then it seems like source.sh doesn't seem like it would do much more than:
for lib in "$LIBRARY"/*.sh; do . "$lib" done
Do you forsee anything more involved that this intermediate file would need/want to do that would make not inlining it meaningful?
Nope. The for loop is fine.
3) Focusing just on the downloads split, how split should we go? It has download_sources which calls download_{local,file,"vcs"} as needed. I was thinking:
$LIBRARY/download.sh /download/local.sh /file.sh /git.sh /...
The download.sh file would source all the files in the download directory and carry the download_sources function.
@Dave: Your input would be particularly helpful here.
Well, it *seems* reasonable. My only concern is how to handle common code (get_url, get_filename, get_filepath) which is called in the individual implementations. These fragments would be incomplete if sourced on their own -- this IMO diminishes the value of separating them out.
Maybe-crazy idea: create a function that wraps the 'source' builtin. Perhaps it would look like:
makepkg_load_module() { local module FUNCNEST=10 if [[ -z ${MAKEPKG_LOADED_MODULES["$1"]} ]]; then unset -v MAKEPKG_MODULE_DEPENDENCIES . "$LIBRARY/$1" MAKEPKG_LOADED_MODULES["$1"]=loaded for module in "${MAKEPKG_MODULE_DEPENDENCIES[@]}"; do makepkg_load_modules "$module" fi fi }
MAKEPKG_LOADED_MODULES would be some associative array we declare before calling this function. modules can declare dependencies which is an array of module names it depends on.
I'm not sure we want to go down this road, but I'm mentioning it anyways. It would certainly, at a minimum, make testing individual modules more straightforward.
This actually is a very good idea - particularly in terms of ease of testing. A module automatically calling all its dependencies would be great. After discussion on IRC, we think an inclusion guard type idea would be a more simple approach. Something like: if (( _PATH_TO_THIS_FILE )); then return; else _PATH_TO_THIS_FILE=1; fi and every file sources all the files containing its needed functions. The first patch I would like to see is: $LIBRARY/util/get_protocol.sh /get_filename.sh ... /download.sh (contains download_sources) /download/file.sh /local.sh /bzr.sh /git.sh /... In fact, make it two patches - the first moving the dependency functions into util, and the second moving the download functions out. Allan