On Mon, Mar 04, 2013 at 10:33:57PM +1000, Allan McRae wrote:

On 28/02/13 13:37, Neer Sighted wrote:

...

I've made the VCS download functions print the correct filename for their clone, as opposed to the last fragment in the url. --- scripts/makepkg.sh.in | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bd29d73..0facb5e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -456,9 +456,7 @@ download_git() { local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")"

- local repo=${netfile##*/} - repo=${repo%%#*} - repo=${repo%%.git*} + local repo=$(get_filename "$netfile")

source=("foobar::git+http://url.com/pacman.git")

Am I downloading the "pacman" repo or the "foobar" repo?

Comments?

I agree that this should be kept as the actual repo, but why not put something like local file=$(get_filename "$netfile") and then when it prints repo, instead print something like msg2 "$(gettext "Cloning %s %s repo to %s...")" "${repo}" "git" "${file}" I think that would solve both issues nicely. Only problem is that in the error messages, it could get a bit long. You could use $dir, and that may be a better solution, as that's already a variable, but that name would be significantly longer for most people. It could be put onto two lines, though: error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git" plain "$(gettext "at %s")" "${dir}" plain "$(gettext "Aborting...")" -- William Giokas | KaiSforza GnuPG Key: 0x73CD09CF Fingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF

This makes the location that VCS repositories are cloned too more obvious, and helps make it more clear what is being cloned, as the last fragment of the URL is not always the most accurate name of a repository. Signed-off-by: Neer Sighted <neersighted@myopera.com> --- scripts/makepkg.sh.in | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bd29d73..5430d5f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -464,10 +464,12 @@ download_git() { url=${url##*git+} url=${url%%#*} + local file=$(get_filename "$netfile") + if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then - msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" + msg2 "$(gettext "Cloning %s %s repo to %s...")" "${repo}" "git" "${file}" if ! git clone --mirror "$url" "$dir"; then - error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git" + error "$(gettext "Failure while cloning %s %s repo (to %s")" "${repo}" "git" "${file}" plain "$(gettext "Aborting...")" exit 1 fi @@ -475,23 +477,23 @@ download_git() { cd_safe "$dir" # Make sure we are fetching the right repo if [[ "$url" != "$(git config --get remote.origin.url)" ]] ; then - error "$(gettext "%s is not a clone of %s")" "$dir" "$url" + error "$(gettext "%s is not a clone of %s")" "${file}" "${url}" plain "$(gettext "Aborting...")" exit 1 fi - msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "git" + msg2 "$(gettext "Updating %s %s repo at %s...")" "${repo}" "git" "${file}" if ! git fetch --all -p; then # only warn on failure to allow offline builds - warning "$(gettext "Failure while updating %s %s repo")" "${repo}" "git" + warning "$(gettext "Failure while updating %s %s repo (at %s")" "${repo}" "git" "${file}" fi fi - msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" "git" + msg2 "$(gettext "Creating working copy of %s %s repo from %s...")" "${repo}" "git" "${file}" pushd "$srcdir" &>/dev/null rm -rf "${dir##*/}" if ! git clone "$dir"; then - error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git" + error "$(gettext "Failure while creating working copy of %s %s repo (from %s")" "${repo}" "git" "${file}" plain "$(gettext "Aborting...")" exit 1 fi @@ -516,7 +518,7 @@ download_git() { if [[ -n $ref ]]; then if ! git checkout -b makepkg $ref; then - error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git" + error "$(gettext "Failure while creating working copy of %s %s repo (from %s)")" "${repo}" "git" "${file}" plain "$(gettext "Aborting...")" exit 1 fi @@ -543,23 +545,25 @@ download_hg() { url=${url##*hg+} url=${url%%#*} + local file=$(get_filename "$netfile") + if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then - msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "hg" + msg2 "$(gettext "Cloning %s %s repo to %s...")" "${repo}" "hg" "${file}" if ! hg clone -U "$url" "$dir"; then - error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "hg" + error "$(gettext "Failure while downloading %s %s repo (to %s")" "${repo}" "hg" "${file}" plain "$(gettext "Aborting...")" exit 1 fi elif (( ! HOLDVER )); then - msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "hg" + msg2 "$(gettext "Updating %s %s repo at %s...")" "${repo}" "hg" "${file}" cd_safe "$dir" if ! hg pull; then # only warn on failure to allow offline builds - warning "$(gettext "Failure while updating %s %s repo")" "${repo}" "hg" + warning "$(gettext "Failure while updating %s %s repo (at %s")" "${repo}" "hg" "${file}" fi fi - msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" "hg" + msg2 "$(gettext "Creating working copy of %s %s repo from %s...")" "${repo}" "hg" "${file}" pushd "$srcdir" &>/dev/null rm -rf "${dir##*/}" @@ -577,7 +581,7 @@ download_hg() { fi if ! hg clone "${ref[@]}" "$dir" "${dir##*/}"; then - error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "hg" + error "$(gettext "Failure while creating working copy of %s %s repo (from %s")" "${repo}" "hg" "${file}" plain "$(gettext "Aborting...")" exit 1 fi @@ -605,23 +609,25 @@ download_svn() { fi url=${url%%#*} + local file=$(get_filename "$netfile") + if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then - msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "svn" + msg2 "$(gettext "Cloning %s %s repo to %s...")" "${repo}" "svn" "${file}" if ! svn checkout --config-dir "$dir" "$url" "$dir"; then - error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "svn" + error "$(gettext "Failure while cloning %s %s repo (to %s")" "${repo}" "svn" "${file}" plain "$(gettext "Aborting...")" exit 1 fi elif (( ! HOLDVER )); then - msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "svn" + msg2 "$(gettext "Updating %s %s repo at %s...")" "${repo}" "svn" "${file}" cd_safe "$dir" if ! svn update; then # only warn on failure to allow offline builds - warning "$(gettext "Failure while updating %s %s repo")" "${repo}" "svn" + warning "$(gettext "Failure while updating %s %s repo (at %s")" "${repo}" "svn" "${file}" fi fi - msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" "svn" + msg2 "$(gettext "Creating working copy of %s %s repo from %s...")" "${repo}" "svn" "${file}" pushd "$srcdir" &>/dev/null rm -rf "${dir##*/}" @@ -639,7 +645,7 @@ download_svn() { fi if ! svn export ${ref[@]} "$dir"; then - error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "svn" + error "$(gettext "Failure while creating working copy of %s %s repo (from %s")" "${repo}" "svn" "${file}" plain "$(gettext "Aborting...")" fi -- 1.8.1.5

On Fri, Mar 08, 2013 at 03:02:53PM -0600, William Giokas wrote:

On Fri, Mar 08, 2013 at 12:31:04PM -0800, Neer Sighted wrote:

...

This makes the location that VCS repositories are cloned too more obvious, and helps make it more clear what is being cloned, as the last fragment of the URL is not always the most accurate name of a repository.

Signed-off-by: Neer Sighted <neersighted@myopera.com> --- scripts/makepkg.sh.in | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bd29d73..5430d5f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -464,10 +464,12 @@ download_git() { url=${url##*git+} url=${url%%#*}

+ local file=$(get_filename "$netfile") + if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then - msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" + msg2 "$(gettext "Cloning %s %s repo to %s...")" "${repo}" "git" "${file}" if ! git clone --mirror "$url" "$dir"; then - error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git" + error "$(gettext "Failure while cloning %s %s repo (to %s")" "${repo}" "git" "${file}" plain "$(gettext "Aborting...")" exit 1 fi @@ -475,23 +477,23 @@ download_git() { cd_safe "$dir" # Make sure we are fetching the right repo if [[ "$url" != "$(git config --get remote.origin.url)" ]] ; then - error "$(gettext "%s is not a clone of %s")" "$dir" "$url" + error "$(gettext "%s is not a clone of %s")" "${file}" "${url}"

I think this should be kept as $dir. I'd like to know the full directory path, not just the filename.

...

plain "$(gettext "Aborting...")" exit 1 fi - msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "git" + msg2 "$(gettext "Updating %s %s repo at %s...")" "${repo}" "git" "${file}"

While $dir may be a bit long, I think if you're going to do "at <blah>" it should be a full path, not just the directory name. You may have to put some '\n's in or us 'plain' to make sure things aren't /too/ long.

...

if ! git fetch --all -p; then # only warn on failure to allow offline builds - warning "$(gettext "Failure while updating %s %s repo")" "${repo}" "git" + warning "$(gettext "Failure while updating %s %s repo (at %s")" "${repo}" "git" "${file}"

Get rid of the () here. They're just unneeded, same for the other three failure messages. Also, use $dir for these. TBH, I now don't think that $file is going to be very useful for this...

Actually, $dir points to the $SRCDEST directory, really. You should probably add file="${srcdir}/${file}" -- William Giokas | KaiSforza GnuPG Key: 0x73CD09CF Fingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF

Make all VCS download functions uses get_filename to get the repo name. In addition, creating a working directory from a Bazaar repository now shows the short-name of the repository, not the full path on disk. I'm not sure if the name of the variable that holds the basename of the local clone should still be `repo`, but I have left the variable name for simplicity. Signed-off-by: Neer Sighted <neersighted@myopera.com> --- scripts/makepkg.sh.in | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index c9661e9..1658091 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -386,6 +386,10 @@ download_bzr() { url=${url##*bzr+} url=${url%%#*} + local dir=$(get_filepath "$netfile") + + local repo=$(get_filename "$netfile") + local fragment=${netfile#*#} if [[ $fragment = "$netfile" ]]; then unset fragment @@ -407,7 +411,6 @@ download_bzr() { esac fi - local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then @@ -432,12 +435,12 @@ download_bzr() { fi fi - msg2 "$(gettext "Creating working copy of %s %s repo...")" "${dir}" "bzr" + msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" "bzr" pushd "$srcdir" &>/dev/null rm -rf "${dir##*/}" if ! bzr checkout "$dir" --lightweight; then - error "$(gettext "Failure while creating working copy of %s %s repo")" "${dir}" "bzr" + error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "bzr" plain "$(gettext "Aborting...")" exit 1 fi @@ -456,9 +459,7 @@ download_git() { local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" - local repo=${netfile##*/} - repo=${repo%%#*} - repo=${repo%%.git*} + local repo=$(get_filename "$netfile") local url=$(get_url "$netfile") url=${url##*git+} @@ -536,8 +537,7 @@ download_hg() { local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" - local repo=${netfile##*/} - repo=${repo%%#*} + local repo=$(get_filename "$netfile") local url=$(get_url "$netfile") url=${url##*hg+} @@ -596,8 +596,7 @@ download_svn() { local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" - local repo=${netfile##*/} - repo=${repo%%#*} + local repo=$(get_filename "$netfile") local url=$(get_url "$netfile") if [[ $url != svn+ssh* ]]; then -- 1.8.1.5

I wanted to keep it more in line with the repo variable definition. Would you like me to put it back? On Sat, Mar 9, 2013, at 02:17 PM, Allan McRae wrote:

On 10/03/13 02:50, Neer Sighted wrote:

...

Make all VCS download functions uses get_filename to get the repo name. In addition, creating a working directory from a Bazaar repository now shows the short-name of the repository, not the full path on disk.

I'm not sure if the name of the variable that holds the basename of the local clone should still be `repo`, but I have left the variable name for simplicity.

Signed-off-by: Neer Sighted <neersighted@myopera.com> --- scripts/makepkg.sh.in | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index c9661e9..1658091 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -386,6 +386,10 @@ download_bzr() { url=${url##*bzr+} url=${url%%#*}

+ local dir=$(get_filepath "$netfile")

Why was that line moved? Otherwise patch is fine.

...

+ + local repo=$(get_filename "$netfile") + local fragment=${netfile#*#} if [[ $fragment = "$netfile" ]]; then unset fragment @@ -407,7 +411,6 @@ download_bzr() { esac fi

- local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")"

if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then @@ -432,12 +435,12 @@ download_bzr() { fi fi

- msg2 "$(gettext "Creating working copy of %s %s repo...")" "${dir}" "bzr" + msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" "bzr" pushd "$srcdir" &>/dev/null rm -rf "${dir##*/}"

if ! bzr checkout "$dir" --lightweight; then - error "$(gettext "Failure while creating working copy of %s %s repo")" "${dir}" "bzr" + error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "bzr" plain "$(gettext "Aborting...")" exit 1 fi @@ -456,9 +459,7 @@ download_git() { local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")"

- local repo=${netfile##*/} - repo=${repo%%#*} - repo=${repo%%.git*} + local repo=$(get_filename "$netfile")

local url=$(get_url "$netfile") url=${url##*git+} @@ -536,8 +537,7 @@ download_hg() { local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")"

- local repo=${netfile##*/} - repo=${repo%%#*} + local repo=$(get_filename "$netfile")

local url=$(get_url "$netfile") url=${url##*hg+} @@ -596,8 +596,7 @@ download_svn() { local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")"

- local repo=${netfile##*/} - repo=${repo%%#*} + local repo=$(get_filename "$netfile")

local url=$(get_url "$netfile") if [[ $url != svn+ssh* ]]; then

-- Neer Sighted http://neersighted.com | neersighted@myopera.com {01DC2056}