[pacman-dev] Bazaar branch check in makepkg
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/e006c48770281be... 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. 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. -- David Macek
On 08.01.2015 23:18, David Macek wrote:
I'm posting this on behalf of an MSYS2 user.
I appreciate you trying to send patches upstream, but you'll likely get more/better responses if you send the patch with git-send-email so it's easy to review/comment on. Also it's probably better if the author sends it and participates in the discussion (unless you really like playing the middleman...). If they don't want the list traffic, that's fine. Mailman has an option so you don't receive any mails, but still post to the list. Just be sure to mention it so people can CC you.
I'm not sure why this kind of code is used but it looks pretty useless for any VCS at all.
commit c926c39b0481ec3db931fff1f86db0c49d78976b quote below
Author: Mohammad Alsaleh <msal@i2pmail.org> Date: Sun Aug 12 00:24:57 2012 +0000
makepkg: check if $dir is a local clone of the right git repo
Before this patch, makepkg does not check if $dir is a local clone of the right repo.
For example, git fetch would be run even if $dir is not a local bare clone of a git repo in present in source(), but a subdir of a checked-out one. That means makepkg can potentially fetch from a completely unrelated remote and update completely unrelated dirs/files.
This patch adds a check to make sure we are fetching from the right remote.
Signed-off-by: Mohammad Alsaleh <msal@i2pmail.org>
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/e006c48770281be...
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
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/e006c48770281be...
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
participants (3)
-
Allan McRae
-
David Macek
-
Florian Pritz