On 11/01/15 16:42, Allan McRae wrote: > On 09/01/15 08:18, David Macek wrote: >> Hi all. >> >> I'm posting this on behalf of an MSYS2 user. We'd like to hear your opinion >> on removing this check from makepkg, which has been causing some troubles >> apparently. >> >> The patch: >> https://raw.githubusercontent.com/renatosilva/MSYS2-packages/e006c48770281be1c952f2063ce0d5ccc4585d86/pacman/0003-Fix-Bazaar-cloning-support-in-makepkg.patch >> >> His comment: >> >> There was some manual check to know if the existing repository is actually a >> clone of the branch specified in $source. However this check needs to be >> semantic, not a simple string comparison. For example, I was blocked from >> building a PKGBUILD which uses a Bazaar repository in $source, because >> Bazaar was returning two different strings for the same location (for HTTP >> one was url-encoded while the other was not, and for local paths one was >> absolute while the other was relative). >> >> While this may be a bug in Bazaar, the check is unreliable since the >> comparison is not semantic (http://foo.com/%2Bplus and http://foo.com/+plus >> obviously refer to the same location for example). It is also useless >> because the intention is updating the existing local clone. However, if the >> local clone is not a real clone of the repository specified in $source >> (which is what this buggy check tries to tell), next step which is a pull >> operation will fail anyway. > > Why would it fail? Given it is in a directory with a VCS PKGBUILD, it > probably is a valid checkout. I quite often switch sources from the > upstream VCS repo, to other peoples copies if they have a development > branch that needs tested. > >> I'm not sure why this kind of code is used but it looks pretty useless for >> any VCS at all. Maybe they wanted to avoid that non-clones successfully >> pulled from $source (for example if the non-clone simply just lacks some >> commits), but isn't this a corner case? Anyway, upstream may be interested >> in removing these checks for all VCS systems. > > I have no interest in removing the check. I do have interest in fixing > the bug... > > Something like this should about do it... > > for (( i = 0; i < length; i++ )); do > local c="${1:i:1}" > case $c in > [a-zA-Z0-9.~_-]) printf "$c" ;; # have I covered everything here? > *) printf '%%%02X' "'$c" > esac > done >
We also need a bug opened to track this. Allan
