[pacman-dev] [PATCH] repo-add: fix db creation one last time
We fubar-ed this pretty good. 1. The whole old/new move shuffle was totally busted if you used a relative path to your database, as we would just build the database in place. 2. Our prior temp directory layout had the database files extracted directly into it. When we tried to create a xxx.db.tar.gz file in this same directory, due to the fact that we were no longer using a shell wildcard, we tried to include the db in ourself, which is a big failure. Fix all this by extracting to tree/ so we can have a clean top-level temp directory. 3. Fix the inclusion of the './' directory entry; ensure the regex prunes both leading paths of '.' as well as './'. Where is that test suite again? Signed-off-by: Dan McGee <dan@archlinux.org> --- scripts/repo-add.sh.in | 20 +++++++++++--------- 1 files changed, 11 insertions(+), 9 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 17b32aa..a845049 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -100,7 +100,7 @@ format_entry() { find_pkgentry() { local pkgname=$1 local pkgentry - for pkgentry in $tmpdir/$pkgname*; do + for pkgentry in $tmpdir/tree/$pkgname*; do name=${pkgentry##*/} if [[ ${name%-*-*} = $pkgname ]]; then echo $pkgentry @@ -285,7 +285,7 @@ db_write_entry() { return 1 fi - pushd "$tmpdir" >/dev/null + pushd "$tmpdir/tree" >/dev/null if [[ -d $pkgname-$pkgver ]]; then warning "$(gettext "An entry for '%s' already existed")" "$pkgname-$pkgver" else @@ -350,7 +350,7 @@ db_write_entry() { # create files file if wanted if (( WITHFILES )); then msg2 "$(gettext "Creating '%s' db entry...")" 'files' - local files_path="$tmpdir/$pkgname-$pkgver/files" + local files_path="$tmpdir/tree/$pkgname-$pkgver/files" echo "%FILES%" >$files_path bsdtar --exclude='^.*' -tf "$pkgfile" >>$files_path fi @@ -381,7 +381,7 @@ db_remove_entry() { while [[ -n $pkgentry ]]; do notfound=0 if [[ -f $pkgentry/deltas ]]; then - mv "$pkgentry/deltas" "$tmpdir/$pkgname.deltas" + mv "$pkgentry/deltas" "$tmpdir/tree/$pkgname.deltas" fi msg2 "$(gettext "Removing existing entry '%s'...")" \ "${pkgentry##*/}" @@ -443,7 +443,7 @@ check_repo_db() { fi verify_signature "$REPO_DB_FILE" msg "$(gettext "Extracting database to a temporary location...")" - bsdtar -xf "$REPO_DB_FILE" -C "$tmpdir" + bsdtar -xf "$REPO_DB_FILE" -C "$tmpdir/tree" else case "$cmd" in repo-remove) @@ -509,7 +509,7 @@ remove() { msg "$(gettext "Searching for package '%s'...")" "$pkgname" if db_remove_entry "$pkgname"; then - rm -f "$tmpdir/$pkgname.deltas" + rm -f "$tmpdir/tree/$pkgname.deltas" return 0 else error "$(gettext "Package matching '%s' not found.")" "$pkgname" @@ -561,6 +561,7 @@ fi tmpdir=$(mktemp -d /tmp/repo-tools.XXXXXXXXXX) || (\ error "$(gettext "Cannot create temp directory for database building.")"; \ exit 1) +mkdir $tmpdir/tree trap 'clean_up' EXIT trap 'trap_exit "$(gettext "TERM signal caught. Exiting...")"' TERM HUP QUIT @@ -628,12 +629,13 @@ if (( success )); then TAR_OPT=$(verify_repo_extension "$REPO_DB_FILE") filename=${REPO_DB_FILE##*/} - pushd "$tmpdir" >/dev/null + pushd "$tmpdir/tree" >/dev/null # strip the './' off filenames; this also allows us to tar an empty dir - bsdtar -s %^./%% -c${TAR_OPT}f "$REPO_DB_FILE" ./ - create_signature "$filename" + bsdtar -s '%^./\?%%' -c${TAR_OPT}f "$tmpdir/$filename" ./ popd >/dev/null + create_signature "$tmpdir/$filename" + [[ -f $REPO_DB_FILE ]] && mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old" [[ -f $REPO_DB_FILE.sig ]] && rm -f "$REPO_DB_FILE.sig" [[ -f $tmpdir/$filename ]] && mv "$tmpdir/$filename" "$REPO_DB_FILE" -- 1.7.6
On 29/06/11 10:58, Dan McGee wrote:
We fubar-ed this pretty good.
1. The whole old/new move shuffle was totally busted if you used a relative path to your database, as we would just build the database in place. 2. Our prior temp directory layout had the database files extracted directly into it. When we tried to create a xxx.db.tar.gz file in this same directory, due to the fact that we were no longer using a shell wildcard, we tried to include the db in ourself, which is a big failure. Fix all this by extracting to tree/ so we can have a clean top-level temp directory. 3. Fix the inclusion of the './' directory entry; ensure the regex prunes both leading paths of '.' as well as './'.
Where is that test suite again?
Signed-off-by: Dan McGee<dan@archlinux.org>
Signed-off-by: Allan Updating my repo without this patch resulted in fun!
On Thu, Jun 30, 2011 at 01:01:35PM +1000, Allan McRae wrote:
On 29/06/11 10:58, Dan McGee wrote:
We fubar-ed this pretty good.
1. The whole old/new move shuffle was totally busted if you used a relative path to your database, as we would just build the database in place. 2. Our prior temp directory layout had the database files extracted directly into it. When we tried to create a xxx.db.tar.gz file in this same directory, due to the fact that we were no longer using a shell wildcard, we tried to include the db in ourself, which is a big failure. Fix all this by extracting to tree/ so we can have a clean top-level temp directory. 3. Fix the inclusion of the './' directory entry; ensure the regex prunes both leading paths of '.' as well as './'.
Where is that test suite again?
Signed-off-by: Dan McGee<dan@archlinux.org>
Signed-off-by: Allan
Updating my repo without this patch resulted in fun!
I think we axed this as well. The tarball gets all out of order (directories first), so we're going to go with my original patch that Dan Nack'ed, which was to use "ugly" bash... (shopt -s nullgob; files=(*); ((${#files[*]}))) almost lispy... d
On Wed, Jun 29, 2011 at 10:06 PM, Dave Reisner <d@falconindy.com> wrote:
On Thu, Jun 30, 2011 at 01:01:35PM +1000, Allan McRae wrote:
On 29/06/11 10:58, Dan McGee wrote:
We fubar-ed this pretty good.
1. The whole old/new move shuffle was totally busted if you used a relative path to your database, as we would just build the database in place. 2. Our prior temp directory layout had the database files extracted directly into it. When we tried to create a xxx.db.tar.gz file in this same directory, due to the fact that we were no longer using a shell wildcard, we tried to include the db in ourself, which is a big failure. Fix all this by extracting to tree/ so we can have a clean top-level temp directory. 3. Fix the inclusion of the './' directory entry; ensure the regex prunes both leading paths of '.' as well as './'.
Where is that test suite again?
Signed-off-by: Dan McGee<dan@archlinux.org>
Signed-off-by: Allan
Updating my repo without this patch resulted in fun!
I think we axed this as well. The tarball gets all out of order (directories first), so we're going to go with my original patch that Dan Nack'ed, which was to use "ugly" bash...
(shopt -s nullgob; files=(*); ((${#files[*]})))
almost lispy...
No, we axed just the bsdtar bit, not the rest of it. Creating a tar file in the same directory as the files you are archiving is not really a good idea ever, even if some crappy shell expansion avoids it. It clearly burned us once, so I'm not having it burn us again. -Dan
On Wed, Jun 29, 2011 at 10:15:12PM -0500, Dan McGee wrote:
On Wed, Jun 29, 2011 at 10:06 PM, Dave Reisner <d@falconindy.com> wrote:
On Thu, Jun 30, 2011 at 01:01:35PM +1000, Allan McRae wrote:
On 29/06/11 10:58, Dan McGee wrote:
We fubar-ed this pretty good.
1. The whole old/new move shuffle was totally busted if you used a relative path to your database, as we would just build the database in place. 2. Our prior temp directory layout had the database files extracted directly into it. When we tried to create a xxx.db.tar.gz file in this same directory, due to the fact that we were no longer using a shell wildcard, we tried to include the db in ourself, which is a big failure. Fix all this by extracting to tree/ so we can have a clean top-level temp directory. 3. Fix the inclusion of the './' directory entry; ensure the regex prunes both leading paths of '.' as well as './'.
Where is that test suite again?
Signed-off-by: Dan McGee<dan@archlinux.org>
Signed-off-by: Allan
Updating my repo without this patch resulted in fun!
I think we axed this as well. The tarball gets all out of order (directories first), so we're going to go with my original patch that Dan Nack'ed, which was to use "ugly" bash...
(shopt -s nullgob; files=(*); ((${#files[*]})))
almost lispy...
No, we axed just the bsdtar bit, not the rest of it. Creating a tar file in the same directory as the files you are archiving is not really a good idea ever, even if some crappy shell expansion avoids it. It clearly burned us once, so I'm not having it burn us again.
-Dan
Oh yeah. We did discuss this. So, ignore the patch I just sent. I've got the right one somewhere... d
On Wed, Jun 29, 2011 at 10:26 PM, Dave Reisner <d@falconindy.com> wrote:
On Wed, Jun 29, 2011 at 10:15:12PM -0500, Dan McGee wrote:
On Wed, Jun 29, 2011 at 10:06 PM, Dave Reisner <d@falconindy.com> wrote:
On Thu, Jun 30, 2011 at 01:01:35PM +1000, Allan McRae wrote:
On 29/06/11 10:58, Dan McGee wrote:
We fubar-ed this pretty good.
1. The whole old/new move shuffle was totally busted if you used a relative path to your database, as we would just build the database in place. 2. Our prior temp directory layout had the database files extracted directly into it. When we tried to create a xxx.db.tar.gz file in this same directory, due to the fact that we were no longer using a shell wildcard, we tried to include the db in ourself, which is a big failure. Fix all this by extracting to tree/ so we can have a clean top-level temp directory. 3. Fix the inclusion of the './' directory entry; ensure the regex prunes both leading paths of '.' as well as './'.
Where is that test suite again?
Signed-off-by: Dan McGee<dan@archlinux.org>
Signed-off-by: Allan
Updating my repo without this patch resulted in fun!
I think we axed this as well. The tarball gets all out of order (directories first), so we're going to go with my original patch that Dan Nack'ed, which was to use "ugly" bash...
(shopt -s nullgob; files=(*); ((${#files[*]})))
almost lispy...
No, we axed just the bsdtar bit, not the rest of it. Creating a tar file in the same directory as the files you are archiving is not really a good idea ever, even if some crappy shell expansion avoids it. It clearly burned us once, so I'm not having it burn us again.
-Dan
Oh yeah. We did discuss this. So, ignore the patch I just sent. I've got the right one somewhere...
I should be able to mash them together in the right fashion. -Dan
Revert to the old behavior that 6f5a90 attempted to simplify and go with the original proposed solution of using "ugly" bash to detect empty directories. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- And here's such a patch... scripts/repo-add.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 17b32aa..37466ea 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -629,8 +629,13 @@ if (( success )); then filename=${REPO_DB_FILE##*/} pushd "$tmpdir" >/dev/null - # strip the './' off filenames; this also allows us to tar an empty dir - bsdtar -s %^./%% -c${TAR_OPT}f "$REPO_DB_FILE" ./ + if ( shopt -s nullglob; files=(*); (( ${#files[*]} )) ); then + bsdtar -c${TAR_OPT}f "$filename" * + else + # we have no packages remaining? zip up some emptyness + warning "$(gettext "No packages remain, creating empty database.")" + bsdtar -c${TAR_OPT}f "$filename" -T /dev/null + fi create_signature "$filename" popd >/dev/null -- 1.7.6
participants (4)
-
Allan McRae
-
Dan McGee
-
Dan McGee
-
Dave Reisner