On Tue, 2013-04-09 at 08:30 -0400, Dave Reisner wrote:
On Tue, Apr 9, 2013 at 5:28 AM, Maxime Gauduin <alucryd@gmail.com> wrote:
From: Alucryd <alucryd@gmail.com>
This check will run 'bzr info' on the distant URL provided in the source array and compare the branch root URL with the output of 'bzr config parent_location' run inside the local repo to make sure we are building from the right sources. Previously, the check was only run locally, as a result the local parent_location had to be used in the source array. makepkg will fallback to this behavior for offline builds.
Signed-off-by: Maxime Gauduin <alucryd@gmail.com> --- scripts/makepkg.sh.in | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d5b9077..0ac4975 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -474,10 +474,21 @@ download_bzr() { fi elif (( ! HOLDVER )); then # Make sure we are fetching the right repo - if [[ "$url" != "$(bzr config parent_location -d $dir)" ]] ; then - error "$(gettext "%s is not a branch of %s")" "$dir" "$url" - plain "$(gettext "Aborting...")" - exit 1 + local distant_url="$(bzr info "$url" 2> /dev/null | grep 'branch root' | sed 's| branch root: ||')"
grep|sed is pretty much always redundant -- sed can do this match on its own:
sed -n '/branch root/ { s/ branch root: //p; q; }'
Thx for the suggestion, somebody suggested the following on the previous thread, which seems to do the trick too: `bzr info lp:lightdm 2> /dev/null | sed -n 's| branch root: ||p'`. It's a bit shorter, if you're okay with it.
And, just nitpicking, but quoting isn't needed for simple variable assignment.
+ local local_url="$(bzr config parent_location -d "$dir")
Same here.
+ if [[ -n "$distant_url" ]]; then
Quoting isn't needed here either.
+ if [[ "$distant_url" != "$local_url" ]]; then
Or on the LHS here.
I apologize if it's a dumb question, but why not the RHS too?
+ error "$(gettext "%s is not a branch of %s")" "$dir" "$url" + plain "$(gettext "Aborting...")" + exit 1 + fi + else + if [[ "$url" != "$local_url" ]] ; then
And same here.
+ error "$(gettext "%s is not a branch of %s")" "$dir" "$url" + error "$(gettext "The local URL is %s")" "$local_url" + plain "$(gettext "Aborting...")" + exit 1 + fi fi msg2 "$(gettext "Pulling %s ...")" "${displaylocation}" cd_safe "$dir" -- 1.8.2
Alright, to sum it up, I have now 3 patches named "Add support for bzr lp URLs in makepkg", "Fix dest dir when using lp: bzr URLs in the source array" (both should be good as is), and this one. Now, would you prefer if I merged the 3 into something like "Full Bazaar support for makepkg" with a full explanation to make it easier for you to merge (and forget about the mess I've made on the mailing list, sorry), or should I just send only this one again with Dave's suggestion? Cheers. -- Maxime