Github messages for voidlinux
 help / color / mirror / Atom feed
* [PR PATCH] [wip] extract distfiles to tmpdir, then move
@ 2021-09-17 21:10 tornaria
  2021-09-17 21:18 ` tornaria
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: tornaria @ 2021-09-17 21:10 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

There is a new pull request by tornaria against master on the void-packages repository

https://github.com/tornaria/void-packages extract-distfiles
https://github.com/void-linux/void-packages/pull/33006

[wip] extract distfiles to tmpdir, then move
This was discussed in irc, and it is a very rough and untested draft.

Right now what the patch does is:
 - extract distfiles into a tmpdir
 - later move to XBPS_BUILDDIR, trying to preserve the same semantics of `wrksrc` and `create_wrksrc` as before.

This should not in practice change the current behaviour.

I also included, commented out, one idea of an alternative version of the "move" step with different semantics:
 - if there's a single directory extracted, move that to XBPS_BUILDDIR renaming to $wrksrc
 - otherwise, move the whole tmpdir into XBPS_BUILDDIR with name $wrksrc.

Actually doing the second step would break a lot of builds...

A patch file from https://github.com/void-linux/void-packages/pull/33006.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-extract-distfiles-33006.patch --]
[-- Type: text/x-diff, Size: 4626 bytes --]

From f85c54a56ae23ce2469f31f822db1ff77953a2c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gonzalo=20Tornar=C3=ADa?= <tornaria@cmat.edu.uy>
Date: Fri, 17 Sep 2021 18:02:25 -0300
Subject: [PATCH] [wip] extract distfiles to tmpdir, then move

---
 common/hooks/do-extract/00-distfiles.sh | 52 +++++++++++++++++--------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/common/hooks/do-extract/00-distfiles.sh b/common/hooks/do-extract/00-distfiles.sh
index 922f7029491a..ea9e6ab04149 100644
--- a/common/hooks/do-extract/00-distfiles.sh
+++ b/common/hooks/do-extract/00-distfiles.sh
@@ -20,10 +20,6 @@ hook() {
 		fi
 	done
 
-	if [ -n "$create_wrksrc" ]; then
-		mkdir -p "${wrksrc}" || msg_error "$pkgver: failed to create wrksrc.\n"
-	fi
-
 	# Disable trap on ERR; the code is smart enough to report errors and abort.
 	trap - ERR
 
@@ -31,6 +27,10 @@ hook() {
 	[ -z "$TAR_CMD" ] && TAR_CMD="$(command -v tar)"
 	[ -z "$TAR_CMD" ] && msg_error "xbps-src: no suitable tar cmd (bsdtar, tar)\n"
 
+	extractdir=$(mktemp -d "$XBPS_BUILDDIR/.extractdir-XXXXXX") &&
+		test -n "$extractdir" && test -d "$extractdir" ||
+		msg_error "$pkgver: failed to create temporary extractdir.\n"
+
 	msg_normal "$pkgver: extracting distfile(s), please wait...\n"
 
 	for f in ${distfiles}; do
@@ -73,17 +73,11 @@ hook() {
 		*) msg_error "$pkgver: unknown distfile suffix for $curfile.\n";;
 		esac
 
-		if [ -n "$create_wrksrc" ]; then
-			extractdir="$wrksrc"
-		else
-			extractdir="$XBPS_BUILDDIR"
-		fi
-
 		case ${cursufx} in
 		tar|txz|tbz|tlz|tgz|crate)
 			$TAR_CMD -x --no-same-permissions --no-same-owner -f $srcdir/$curfile -C "$extractdir"
 			if [ $? -ne 0 ]; then
-				msg_error "$pkgver: extracting $curfile into $XBPS_BUILDDIR.\n"
+				msg_error "$pkgver: extracting $curfile into $extractdir.\n"
 			fi
 			;;
 		gz|bz2|xz)
@@ -105,12 +99,12 @@ hook() {
 			if command -v unzip &>/dev/null; then
 				unzip -o -q $srcdir/$curfile -d "$extractdir"
 				if [ $? -ne 0 ]; then
-					msg_error "$pkgver: extracting $curfile into $XBPS_BUILDDIR.\n"
+					msg_error "$pkgver: extracting $curfile into $extractdir.\n"
 				fi
 			elif command -v bsdtar &>/dev/null; then
 				bsdtar -xf $srcdir/$curfile -C "$extractdir"
 				if [ $? -ne 0 ]; then
-					msg_error "$pkgver: extracting $curfile into $XBPS_BUILDDIR.\n"
+					msg_error "$pkgver: extracting $curfile into $extractdir.\n"
 				fi
 			else
 				msg_error "$pkgver: cannot find unzip or bsdtar bin for extraction.\n"
@@ -121,7 +115,7 @@ hook() {
 				cd "$extractdir"
 				rpmextract $srcdir/$curfile
 				if [ $? -ne 0 ]; then
-					msg_error "$pkgver: extracting $curfile into $XBPS_BUILDDIR.\n"
+					msg_error "$pkgver: extracting $curfile into $extractdir.\n"
 				fi
 			else
 				msg_error "$pkgver: cannot find rpmextract for extraction.\n"
@@ -138,12 +132,12 @@ hook() {
 			if command -v 7z &>/dev/null; then
 				7z x $srcdir/$curfile -o"$extractdir"
 				if [ $? -ne 0 ]; then
-					msg_error "$pkgver: extracting $curfile into $XBPS_BUILDDIR.\n"
+					msg_error "$pkgver: extracting $curfile into $extractdir.\n"
 				fi
 			elif command -v bsdtar &>/dev/null; then
 				bsdtar -xf $srcdir/$curfile -C "$extractdir"
 				if [ $? -ne 0 ]; then
-					msg_error "$pkgver: extracting $curfile into $XBPS_BUILDDIR.\n"
+					msg_error "$pkgver: extracting $curfile into $extractdir.\n"
 				fi
 			else
 				msg_error "$pkgver: cannot find 7z or bsdtar bin for extraction.\n"
@@ -161,7 +155,7 @@ hook() {
 					;;
 			esac
 			if [ $? -ne 0 ]; then
-				msg_error "$pkgver: extracting $curfile into $XBPS_BUILDDIR.\n"
+				msg_error "$pkgver: extracting $curfile into $extractdir.\n"
 			fi
 			;;
 		*)
@@ -169,4 +163,28 @@ hook() {
 			;;
 		esac
 	done
+
+	### CURRENT BEHAVIOUR is roughly equivalent to this:
+	if [ -n "$create_wrksrc" ]; then
+		mkdir -p "$wrksrc" || msg_error "$pkgver: failed to create wrksrc.\n"
+		echo mv -i "$extractdir"/* -t "$wrksrc" ||
+			msg_error "$pkgver: failed to move $extractdir/* into $wrksrc"
+	else
+		echo mv -i "$extractdir"/* -t "$XBPS_BUILDDIR" ||
+			msg_error "$pkgver: failed to move $extractdir/* into $XBPS_BUILDIR"
+	fi
+
+	# ### A slightly different (more sensible?) behaviour, ignoring create_wrksrc:
+	# set -- "$extractdir"/*
+
+	# if [ "$#" -eq 1 ] && [ -d "$1" ]; then
+	# 	mv -i "$1" -T "$wrksrc" ||
+	# 		msg_error "$pkgver: failed to move '$1' to '$wrksrc'"
+	# else
+	# 	mkdir -p "$wrksrc" || msg_error "$pkgver: failed to create wrksrc.\n"
+	# 	mv -i "$@" -t "$wrksrc" ||
+	# 		msg_error "$pkgver: failed to move '$extractdir/*' into '$wrksrc'"
+	# fi
+
+	msg_error "abort to inspect\n"
 }

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [wip] extract distfiles to tmpdir, then move
  2021-09-17 21:10 [PR PATCH] [wip] extract distfiles to tmpdir, then move tornaria
@ 2021-09-17 21:18 ` tornaria
  2021-09-17 21:20 ` tornaria
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: tornaria @ 2021-09-17 21:18 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

New comment by tornaria on void-packages repository

https://github.com/void-linux/void-packages/pull/33006#issuecomment-922087250

Comment:
To help the conversation, this affects the do_extract hook in the following way:
 1. The first step is to create a tmpdir named `"$XBPS_BUILDDIR/.extractdir-XXXXXX"`, we call that `$extractdir`
 2. The second step is the actual extraction, which is just as before but extracting everything into `$extractdir`
 3. The third step is the move, so that's where the semantics lies.

The move, with current behaviour, I think would be something like
```
if [ -n "$create_wrksrc" ]; then
	mkdir -p "$wrksrc" || msg_error "$pkgver: failed to create wrksrc.\n"
	echo mv -i "$extractdir"/* -t "$wrksrc" ||
		msg_error "$pkgver: failed to move $extractdir/* into $wrksrc"
else
	echo mv -i "$extractdir"/* -t "$XBPS_BUILDDIR" ||
		msg_error "$pkgver: failed to move $extractdir/* into $XBPS_BUILDIR"
fi
```

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [wip] extract distfiles to tmpdir, then move
  2021-09-17 21:10 [PR PATCH] [wip] extract distfiles to tmpdir, then move tornaria
  2021-09-17 21:18 ` tornaria
@ 2021-09-17 21:20 ` tornaria
  2021-09-17 22:10 ` q66
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: tornaria @ 2021-09-17 21:20 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

New comment by tornaria on void-packages repository

https://github.com/void-linux/void-packages/pull/33006#issuecomment-922088123

Comment:
Here different semantics for the move step:
```
set -- "$extractdir"/*
if [ "$#" -eq 1 ] && [ -d "$1" ]; then
	mv -i "$1" -T "$wrksrc" ||
		msg_error "$pkgver: failed to move '$1' to '$wrksrc'"
else
	mkdir -p "$wrksrc" || msg_error "$pkgver: failed to create wrksrc.\n"
	mv -i "$@" -t "$wrksrc" ||
		msg_error "$pkgver: failed to move '$extractdir/*' into '$wrksrc'"
fi
```
More precisely:
 - if there's a single directory extracted, move that to XBPS_BUILDDIR renaming to `$wrksrc`
 - otherwise, move the whole tmpdir into XBPS_BUILDDIR with name `$wrksrc`.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [wip] extract distfiles to tmpdir, then move
  2021-09-17 21:10 [PR PATCH] [wip] extract distfiles to tmpdir, then move tornaria
  2021-09-17 21:18 ` tornaria
  2021-09-17 21:20 ` tornaria
@ 2021-09-17 22:10 ` q66
  2021-09-30  2:12 ` tornaria
  2021-09-30  2:12 ` [PR PATCH] [Closed]: " tornaria
  4 siblings, 0 replies; 6+ messages in thread
From: q66 @ 2021-09-17 22:10 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 200 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/33006#issuecomment-922108550

Comment:
okay, but you failed to mention what the point of doing this is

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [wip] extract distfiles to tmpdir, then move
  2021-09-17 21:10 [PR PATCH] [wip] extract distfiles to tmpdir, then move tornaria
                   ` (2 preceding siblings ...)
  2021-09-17 22:10 ` q66
@ 2021-09-30  2:12 ` tornaria
  2021-09-30  2:12 ` [PR PATCH] [Closed]: " tornaria
  4 siblings, 0 replies; 6+ messages in thread
From: tornaria @ 2021-09-30  2:12 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 203 bytes --]

New comment by tornaria on void-packages repository

https://github.com/void-linux/void-packages/pull/33006#issuecomment-930692594

Comment:
Closing in favor of #33013, which is more advanced than this.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PR PATCH] [Closed]: [wip] extract distfiles to tmpdir, then move
  2021-09-17 21:10 [PR PATCH] [wip] extract distfiles to tmpdir, then move tornaria
                   ` (3 preceding siblings ...)
  2021-09-30  2:12 ` tornaria
@ 2021-09-30  2:12 ` tornaria
  4 siblings, 0 replies; 6+ messages in thread
From: tornaria @ 2021-09-30  2:12 UTC (permalink / raw)
  To: ml

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]

There's a closed pull request on the void-packages repository

[wip] extract distfiles to tmpdir, then move
https://github.com/void-linux/void-packages/pull/33006

Description:
This was discussed in irc, and it is a very rough and untested draft.

Right now what the patch does is:
 - extract distfiles into a tmpdir
 - later move to XBPS_BUILDDIR, trying to preserve the same semantics of `wrksrc` and `create_wrksrc` as before.

This should not in practice change the current behaviour.

I also included, commented out, one idea of an alternative version of the "move" step with different semantics:
 - if there's a single directory extracted, move that to XBPS_BUILDDIR renaming to $wrksrc
 - otherwise, move the whole tmpdir into XBPS_BUILDDIR with name $wrksrc.

Actually doing the second step would break a lot of builds...

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-30  2:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 21:10 [PR PATCH] [wip] extract distfiles to tmpdir, then move tornaria
2021-09-17 21:18 ` tornaria
2021-09-17 21:20 ` tornaria
2021-09-17 22:10 ` q66
2021-09-30  2:12 ` tornaria
2021-09-30  2:12 ` [PR PATCH] [Closed]: " tornaria

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).