[pacman-dev] [PATCH 0/5] Splitting up makepkg into libmakepkg

[Allan McRae]

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