FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 02-02-2008, 11:18 PM
Chantry Xavier
 
Default Clarify the "failed to add target" errors.

Make the error message printed when addtarget fails consistent between
add.c, remove.c and sync.c.

The main problem was that the "failed to add target" in case of a removal
operation could sound confusing.
There was also a little output problem with -U ("failed" was missing).

Signed-off-by: Chantry Xavier <shiningxc@gmail.com>
---
src/pacman/add.c | 5 +++--
src/pacman/remove.c | 4 ++--
src/pacman/sync.c | 2 +-
3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/pacman/add.c b/src/pacman/add.c
index 975d26b..debe5c4 100644
--- a/src/pacman/add.c
+++ b/src/pacman/add.c
@@ -115,8 +115,9 @@ int pacman_add(alpm_list_t *targets)
for(i = targets; i; i = alpm_list_next(i)) {
char *targ = alpm_list_getdata(i);
if(alpm_trans_addtarget(targ) == -1) {
- fprintf(stderr, _("error: failed to add target '%s' (%s)"), targ,
- alpm_strerrorlast());
+ printf("failed.
");
+ fprintf(stderr, _("error: '%s': %s
"),
+ targ, alpm_strerrorlast());
add_cleanup();
return(1);
}
diff --git a/src/pacman/remove.c b/src/pacman/remove.c
index 56837fa..0722057 100644
--- a/src/pacman/remove.c
+++ b/src/pacman/remove.c
@@ -108,8 +108,8 @@ int pacman_remove(alpm_list_t *targets)
char *targ = alpm_list_getdata(i);
if(alpm_trans_addtarget(targ) == -1) {
printf("failed.
");
- fprintf(stderr, _("error: failed to add target '%s' (%s)
"), targ,
- alpm_strerrorlast());
+ fprintf(stderr, _("error: '%s': %s
"),
+ targ, alpm_strerrorlast());
remove_cleanup();
FREELIST(finaltargs);
return(1);
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 582192a..27218d6 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -568,7 +568,7 @@ static int sync_trans(alpm_list_t *targets, int sync_only)
}
if(pm_errno != PM_ERR_PKG_NOT_FOUND) {
fprintf(stderr, _("error: '%s': %s
"),
- (char *)i->data, alpm_strerrorlast());
+ targ, alpm_strerrorlast());
retval = 1;
goto cleanup;
}
--
1.5.4


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 02-04-2008, 03:00 PM
Nagy Gabor
 
Default Clarify the "failed to add target" errors.

Hi!

Well, this error message problem is not critical at all, but I don't like your
preferred fix, because:
- all "transaction steps" (init, release, addtarget, prepare, commit) use the
same format now: "error: failed to foo transaction (ERRORLAST)
" and the patch
would break this simmetry.
- I'm not sure that alpm_strerrorlast() is always informative and set properly
- we can get the same ERRORLAST so foo can be helpful in debugging in these cases
- Personally I liked this front-end + back-end error message "concatenation"

So I prefer rephase this "failed to add target" message, Dan's suggestion seems
OK to me: http://www.archlinux.org/pipermail/pacman-dev/2008-February/011045.html

Bye


----------------------------------------------------
SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu
This mail sent through IMP: http://horde.org/imp/


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 02-04-2008, 05:03 PM
Xavier
 
Default Clarify the "failed to add target" errors.

Nagy Gabor wrote:
> Hi!
>
> Well, this error message problem is not critical at all, but I don't like your
> preferred fix, because:
> - all "transaction steps" (init, release, addtarget, prepare, commit) use the
> same format now: "error: failed to foo transaction (ERRORLAST)
" and the patch
> would break this simmetry.


I simply kept the sync.c message, and used the same for remove.c and
add.c. So on this side, my patch makes it more consistent.
Maybe sync.c wasn't consistent in the first place, but well :P

> - I'm not sure that alpm_strerrorlast() is always informative and set properly

Then make it more informative

> - we can get the same ERRORLAST so foo can be helpful in debugging in these cases
> - Personally I liked this front-end + back-end error message "concatenation"
>
> So I prefer rephase this "failed to add target" message, Dan's suggestion seems
> OK to me: http://www.archlinux.org/pipermail/pacman-dev/2008-February/011045.html
>

So should I change in my patch from :
fprintf(stderr, _("error: '%s': %s
"),
targ, alpm_strerrorlast());
to
fprintf(stderr, _("error: failed to find target '%s' (%s)
"),
targ, alpm_strerrorlast());

for sync.c , remove.c and add.c ?

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 02-05-2008, 12:21 PM
Nagy Gabor
 
Default Clarify the "failed to add target" errors.

>
> So should I change in my patch from :
> fprintf(stderr, _("error: '%s': %s
"),
> targ, alpm_strerrorlast());
> to
> fprintf(stderr, _("error: failed to find target '%s' (%s)
"),
> targ, alpm_strerrorlast());
>
> for sync.c , remove.c and add.c ?
>

my vote: yes
[Note: you should patch line 632 in sync.c; and may rephrase the other error
messages around this step1 part.]

Bye


----------------------------------------------------
SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu
This mail sent through IMP: http://horde.org/imp/


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 02-05-2008, 12:41 PM
Xavier
 
Default Clarify the "failed to add target" errors.

Nagy Gabor wrote:
>> So should I change in my patch from :
>> fprintf(stderr, _("error: '%s': %s
"),
>> targ, alpm_strerrorlast());
>> to
>> fprintf(stderr, _("error: failed to find target '%s' (%s)
"),
>> targ, alpm_strerrorlast());
>>
>> for sync.c , remove.c and add.c ?
>>
>
> my vote: yes
> [Note: you should patch line 632 in sync.c; and may rephrase the other error
> messages around this step1 part.]
>

Actually, that message would not be acceptable in sync.c, line 570.

569 if(pm_errno != PM_ERR_PKG_NOT_FOUND) {
570 fprintf(stderr, _("error: failed to find target '%s'
"),
571 (char *)i->data, alpm_strerrorlast());

This error is not displayed when the package is not found, but when
something else happens.

So I am not going to change my previous patch. I will let Dan choose
whether he accepts it or not (he already pulled it in his maint branch :
http://code.toofishes.net/gitweb.cgi?p=pacman.git;a=shortlog;h=maint).

In any cases (whether my patch is applied or not), you are free to
propose another patch, since you are apparently not satisfied.

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

Thread Tools




All times are GMT. The time now is 10:44 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright ©2007 - 2008, www.linux-archive.org