silvio@port1024.net wrote:
> From: Silvio fricke<silvio@port1024.net>
>
> Signed-off-by: Silvio fricke<silvio@port1024.net>
> ---
> scripts/makepkg.sh.in | 58 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 57 insertions(+), 1 deletions(-)
>

I have only minor "appearance" issues to address here, I don't know what
to think about the patch itself. Only that it does not seem too big and
mostly concentrated in one place, which is good.

So first, isn't the correct term "splitted"?
It seems that you use both "split" and "splittet" which would then be
incorrect.

Also you should read this :
http://archlinux.org/pacman/submitting-patches.html
Especially this part :
<<
Describe your patch.

Before the actual diff begins, it helps if you describe the changes in
the patch. This allows others to see what you intended so as to compare
it to what was actually done, and allows better feedback. If you use
git-format-patch to create your patch, then your commit message will be
shown above the patch by default.
>>

Also it is rather strange that there is no space between your name and
mail address. Did you do something like the above :
git repo-config user.name "Your Name"
git repo-config user.email "me@example.com"


> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 5bdc5bc..3c0c6f7 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -675,6 +675,8 @@ run_build() {
> mv "$BUILDLOG" "$BUILDLOG.$i"
> fi
>
> + export BUILDLOG
> +
> build 2>&1 | tee "$BUILDLOG"; ret=${PIPESTATUS[0]}
> else
> build 2>&1 || ret=$?
> @@ -1389,7 +1391,57 @@ if [ "$INFAKEROOT" = "1" ]; then
> tidy_install
> fi
>
> - create_package
> + if [ "$splitinstall" != "" ] ; then
> + backup_o=$backup
> + conflicts_o=$conflicts
> + depends_o=$depends
> + groups_o=$groups
> + install_o=$install
> + license_o=$license
> + pkgdesc_o=$pkgdesc
> + pkgname_o=$pkgname
> + pkgver_o=$pkgver
> + provides_o=$provides
> + replaces_o=$replaces
> + url_o=$url
> + create_package
> + for it in "${splitinstall[@]}" ; do
> + if [ -d "$pkgdir" ]; then
> + msg "Removing existing pkg/ directory..."
> + rm -rf "$pkgdir"
> + fi
> + mkdir -p "$pkgdir"
> + msg "Building split package using function: $it"
> +

splitted?

> + if [ "$LOGGING" = "1" ]; then
> + [ "x$BUILDLOG" == "x" ]&& BUILDLOG="${startdir}/${pkgname}-${pkgver}-${pkgrel}-${CARCH}.log"
> + echo>> "$BUILDLOG"
> + echo>> "$BUILDLOG"
> + echo "--- Logging for $it ------------">> "$BUILDLOG"
> +

What about wraiden's suggestion in the bug comments :
<<
You could change the comment appending into a single line of the
following form which would be cleaner in my opinion.

echo -e "

--- Logging for $it ------------" >> "$BUILDLOG"
>>


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev

Miklos Vajna wrote:
> On Sat, Jun 14, 2008 at 09:06:23PM +0200, Xavier<shiningxc@gmail.com> wrote:
>> So first, isn't the correct term "splitted"?
>> It seems that you use both "split" and "splittet" which would then be
>> incorrect.
>
> i'm not native, but: split, split, split. splitted is not an English
> term at all.
>

To be honest I wasn't sure at all between split and splitted Only
that splittet was wrong but probably a typo for splitted.
The fact that google returned one million results for splitted didn't
help me though, but it indeed seems that split is the correct one.

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev

silvio@port1024.net wrote:
> <snip>
>
> + backup_o=$backup
> + conflicts_o=$conflicts
> + depends_o=$depends
> + groups_o=$groups
> + install_o=$install
> + license_o=$license
> + pkgdesc_o=$pkgdesc
> + pkgname_o=$pkgname
> + pkgver_o=$pkgver
> + provides_o=$provides
> + replaces_o=$replaces
> + url_o=$url
> <snip>
> + [ "$backup_o" != "" ] && backup=$backup_o || unset backup
> + [ "$conflicts_o" != "" ] && conflicts=$conflicts_o || unset conflicts
> + [ "$depends_o" != "" ] && depends=$depends_o || unset depends
> + [ "$groups_o" != "" ] && groups=$groups_o || unset groups
> + [ "$install_o" != "" ] && install=$install_o || unset install
> + [ "$license_o" != "" ] && license=$license_o || unset license
> + [ "$pkgdesc_o" != "" ] && pkgdesc=$pkgdesc_o || unset pkgdesc
> + [ "$pkgname_o" != "" ] && pkgname=$pkgname_o || unset pkgname
> + [ "$pkgver_o" != "" ] && pkgver=$pkgver_o || unset pkgver
> + [ "$provides_o" != "" ] && provides=$provides_o || unset provides
> + [ "$replaces_o" != "" ] && replaces=$replaces_o || unset replaces
> + [ "$url_o" != "" ] && url=$url_o || unset url
> <snip>
>

I think it would be cleaner to get these in a loop and in the same order
as in PKGBUILD.proto.

As I said in another email about splitting packages, I really don't like
the whole deleting the pkgdir at the end of packaging each split
package. However, in general I think this approach to splitting packages
is OK.

Allan



_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev

Hello,

> Also it is rather strange that there is no space between your name and
> mail address. Did you do something like the above :
> git repo-config user.name "Your Name"
> git repo-config user.email "me@example.com"

> silvio@port1024.net wrote:
> > From: Silvio fricke<silvio@port1024.net>

Very strange, because [1] show me that I have spaces between my name and
address.

> I have only minor "appearance" issues to address here, I don't know what
> to think about the patch itself. Only that it does not seem too big and
> mostly concentrated in one place, which is good.

Ok, fine!

> So first, isn't the correct term "splitted"?
> It seems that you use both "split" and "splittet" which would then be
> incorrect.

Hmm, english is not my native language and I need help in language
things.
But I have the suggestions from [2] used.

> Also you should read this :
> http://archlinux.org/pacman/submitting-patches.html
> Especially this part :
> <<
> Describe your patch.
>
> Before the actual diff begins, it helps if you describe the changes in
> the patch. This allows others to see what you intended so as to compare
> it to what was actually done, and allows better feedback. If you use
> git-format-patch to create your patch, then your commit message will be
> shown above the patch by default.
> >>

Right you are! The next version of this patch has a more descriptive
description.

> > + if [ "$LOGGING" = "1" ]; then
> > + [ "x$BUILDLOG" == "x" ]&& BUILDLOG="${startdir}/${pkgname}-${pkgver}-${pkgrel}-${CARCH}.log"
> > + echo>> "$BUILDLOG"
> > + echo>> "$BUILDLOG"
> > + echo "--- Logging for $it ------------">> "$BUILDLOG"
> > +
>
> What about wraiden's suggestion in the bug comments :
> <<
> You could change the comment appending into a single line of the
> following form which would be cleaner in my opinion.
>
> echo -e "

--- Logging for $it ------------" >> "$BUILDLOG"
> >>

I have it beautified!


[1] http://archlinux.org/pipermail/pacman-dev/2008-June/012133.html
[2] http://archlinux.org/pipermail/pacman-dev/2008-June/012147.html

Bye eS.eF.

--
-- S. Fricke ---------------------------------------- silvio@port1024.net --
Diplom-Informatiker (FH)
Linux-Entwicklung JABBER: silvio@conversation.port1024.net
----------------------------------------------------------------------------


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev

Xavier wrote:
>> Also it is rather strange that there is no space between your name and
>> mail address. Did you do something like the above :
>> git repo-config user.name "Your Name"
>> git repo-config user.email "me@example.com"
>
>> silvio@port1024.net wrote:
>>> From: Silvio fricke<silvio@port1024.net>
>
> Very strange, because [1] show me that I have spaces between my name and
> address.
>

Oops, seems to be a thunderbird issue when replying.

>> I have only minor "appearance" issues to address here, I don't know what
>> to think about the patch itself. Only that it does not seem too big and
>> mostly concentrated in one place, which is good.
>
> Ok, fine!
>
>> So first, isn't the correct term "splitted"?
>> It seems that you use both "split" and "splittet" which would then be
>> incorrect.
>
> Hmm, english is not my native language and I need help in language
> things.
> But I have the suggestions from [2] used.
>

Yeah, use split.

>> Also you should read this :
>> http://archlinux.org/pacman/submitting-patches.html
>> Especially this part :
>> <<
>> Describe your patch.
>>
>> Before the actual diff begins, it helps if you describe the changes in
>> the patch. This allows others to see what you intended so as to compare
>> it to what was actually done, and allows better feedback. If you use
>> git-format-patch to create your patch, then your commit message will be
>> shown above the patch by default.
>> >>
>
> Right you are! The next version of this patch has a more descriptive
> description.
>

Good.
Also have a look at Allan's suggestion. Seems like the whole patch would
be nicer if it could use a loop.

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev

On Sun, 15 Jun 2008 19:42:49 +1000
Allan McRae <mcrae_allan@hotmail.com> wrote:

> I think it would be cleaner to get these in a loop and in the same order
> as in PKGBUILD.proto.
>
> As I said in another email about splitting packages, I really don't like
> the whole deleting the pkgdir at the end of packaging each split
> package. However, in general I think this approach to splitting packages
> is OK.

Yeah I'd rather not see pkgdir deleted either. It eliminates the
possibility of just using --repackage to fix some minor mistakes.

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev