On 03/12/2012 01:13 PM, Dave Reisner wrote:
> On Mon, Mar 12, 2012 at 12:53:11PM -0600, dgbale...@0x01b.net wrote:
>> From: Matthew Monaco <matthew.mon...@0x01b.net>
>>
>> Rather than prioritizing an arbitrary VCS, collect all development
>> directives. If there is more than one, use the package name as a hint.
>> If that doesn't work, abort.
>> ---
> 
> I'm not really sure I understand the need for this. In what use case
> are multiple VCS definitions needed?
> 

I've only seen multiples when a project is transitioning to a new system. Either
way, I think that splitting the check for the which vcs is being used and
setting the new version might be useful.

>>  scripts/makepkg.sh.in |   21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>> index 05a611d..1eb62ca 100644
>> --- a/scripts/makepkg.sh.in
>> +++ b/scripts/makepkg.sh.in
>> @@ -1714,6 +1714,27 @@ devel_check() {
>>              # calls to makepkg via fakeroot will explicitly pass the version
>>              # number to avoid having to determine the version number twice.
>>              # Also do a check to make sure we have the VCS tool available.
>> +            local vcs=()
>> +
>> +            [[ -n ${_darcstrunk} && -n ${_darcsmod} ]] && vcs+=("darcs")
>> +            [[ -n ${_cvsroot}    && -n ${_cvsmod}   ]] && vcs+=("cvs")
>> +            [[ -n ${_gitroot}    && -n ${_gitname}  ]] && vcs+=("git")
>> +            [[ -n ${_svntrunk}   && -n ${_svnmod}   ]] && vcs+=("svn")
>> +            [[ -n ${_bzrtrunk}   && -n ${_bzrmod}   ]] && vcs+=("bzr")
>> +            [[ -n ${_hgroot}     && -n ${_hgrepo}   ]] && vcs+=("hg")
>> +
>> +            if [[ ${#vcs[@]} -eq 0 ]]; then
> 
> We generally use arithmetic contexts elsewhere for numeric comparison.
>

ok

>> +                    return
>> +            elif [[ ${#vcs[@]} -ge 2 ]]; then
>> +                    local vcslen=${#vcs[@]}
>> +                    vcs=(${vcs[@]/${pkgname##*-}/})
> 
> I think you're intentionally performing whitespace splitting here to
> avoid counting problems. Even if that isn't the reason, it's not kosher.
> 

it's a quick way to test if the -suffix matches the two _vcs variables. also,
the potential list is confined to what is above and a vcs with a space in the
name isn't likely.

>> +                    if [[ ${#vcs[@]} -eq $vcslen ]]; then
>> +                            warning "$(gettext "Ambiguous VCS package. 
>> Cannot pick from: %s.")" "${vcs[*]}"
>> +                            return 0
>> +                    fi
>> +                    vcs=${pkgname##*-}
> 
> And after all that array business, vcs is now treated as a string. I
> appreciate bash's "dynamic" typing, but I rarely seek to abuse it.
>

ok

>> +            fi
>> +
>>              if [[ -n ${_darcstrunk} && -n ${_darcsmod} ]] ; then
>>                      if ! type -p darcs >/dev/null; then
>>                              warning "$(gettext "Cannot find the %s binary 
>> required to determine latest %s revision.")" "darcs" "darcs"
>> -- 
>> 1.7.9.3
>>
>>
> 


Reply via email to