On Tue, Apr 9, 2013 at 6:29 PM, Dave Reisner <[email protected]> wrote:
> On Tue, Apr 9, 2013 at 9:25 AM, Maxime Gauduin <[email protected]> wrote: > > > > On Tue, 2013-04-09 at 08:30 -0400, Dave Reisner wrote: > > > On Tue, Apr 9, 2013 at 5:28 AM, Maxime Gauduin <[email protected]> > wrote: > > > > > > > From: Alucryd <[email protected]> > > > > > > > > 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 <[email protected]> > > > > --- > > > > 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. > > Well, its shorter, but its not necessarily equivalent. If "branch root" > appears on two different lines, the shorter version prints twice. I'm not > sure that this would ever happen, but better to be safe, imo. > It should not but I agree, I'll use the version you provided then. > > > > > > 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? > > glob metachars will be interpreted on the RHS, but the LHS is always taken > as literal. It allows for matching: > > [[ foobar = foo* ]] # this is true > [[ foo[a-z]bar = "foo[a-z]bar" ]] # this is true > [[ foo[a-z]bar = foo[a-z]bar ]] # this is false > [[ foobar = "foo*" ]] # this is false > > In single argument tests, there's never any interpretation and strings are > always treated as literals. > I see, thank you for the kind explanation. > > > > > > > > > > + 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? > > No mess. Just send another version. Though, I suggest versioning your > patches and threading: > > [PATCH] I added an awesome new feature > > (review happens) > > [PATCHv2] I added an awesome new feature > > (This patch is in-reply-to the message-id of the original patch. more > review happens, perhaps) > > [PATCHv3] I added an awesome new feature > > (This patch is in-reply-to the message-id of patchv2) > > Sure, I'll try to send a v2 patch tomorrow then, merging the 2 others into this one. No need to fragment since they're tiny. Thanks again. -- Maxime
