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 06-24-2011, 04:06 AM
Dan McGee
 
Default repo-add: avoid using ls to check for presence of files

On Wed, Jun 22, 2011 at 7:38 PM, Dave Reisner <d@falconindy.com> wrote:

I agree with the commit message, but damn is this ugly. Can't we drop
the whole nullglob bit and just do something like:

contents=*
if [[ $contents = "*" ]]; then

?

> Signed-off-by: Dave Reisner <d@falconindy.com>
> ---
> *scripts/repo-add.sh.in | * *2 +-
> *1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> index 029e17d..e75055f 100644
> --- a/scripts/repo-add.sh.in
> +++ b/scripts/repo-add.sh.in
> @@ -593,7 +593,7 @@ if (( success )); then
> * * * *filename=${REPO_DB_FILE##*/}
>
> * * * *pushd "$tmpdir" >/dev/null
> - * * * if [[ -n $(ls) ]]; then
> + * * * if ( shopt -s nullglob; files=(*); (( ${#files[*]} )) ); then
> * * * * * * * *bsdtar -c${TAR_OPT}f "$filename" *
> * * * *else
> * * * * * * * *# we have no packages remaining? zip up some emptyness
> --
> 1.7.5.4
>
>
>
 
Old 06-24-2011, 04:34 AM
Dave Reisner
 
Default repo-add: avoid using ls to check for presence of files

On Thu, Jun 23, 2011 at 11:06:45PM -0500, Dan McGee wrote:
> On Wed, Jun 22, 2011 at 7:38 PM, Dave Reisner <d@falconindy.com> wrote:
>
> I agree with the commit message, but damn is this ugly. Can't we drop
> the whole nullglob bit and just do something like:
>
> contents=*
> if [[ $contents = "*" ]]; then
>
> ?
>

The * won't expand when assigned to a variable, only to an array. So, in
theory:

contents=(*)
if [[ $contents = "*" ]]; then

But the nullglob is safer. I'm pretty sure almost _anything_ that's
considered best practice in Bash is going to be ugly by your standards.

d

> > Signed-off-by: Dave Reisner <d@falconindy.com>
> > ---
> > *scripts/repo-add.sh.in | * *2 +-
> > *1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> > index 029e17d..e75055f 100644
> > --- a/scripts/repo-add.sh.in
> > +++ b/scripts/repo-add.sh.in
> > @@ -593,7 +593,7 @@ if (( success )); then
> > * * * *filename=${REPO_DB_FILE##*/}
> >
> > * * * *pushd "$tmpdir" >/dev/null
> > - * * * if [[ -n $(ls) ]]; then
> > + * * * if ( shopt -s nullglob; files=(*); (( ${#files[*]} )) ); then
> > * * * * * * * *bsdtar -c${TAR_OPT}f "$filename" *
> > * * * *else
> > * * * * * * * *# we have no packages remaining? zip up some emptyness
> > --
> > 1.7.5.4
> >
> >
> >
>
 
Old 06-24-2011, 01:43 PM
Martti Kühne
 
Default repo-add: avoid using ls to check for presence of files

On Fri, Jun 24, 2011 at 6:34 AM, Dave Reisner <d@falconindy.com> wrote:
> On Thu, Jun 23, 2011 at 11:06:45PM -0500, Dan McGee wrote:
>> On Wed, Jun 22, 2011 at 7:38 PM, Dave Reisner <d@falconindy.com> wrote:
>>
>> I agree with the commit message, but damn is this ugly. Can't we drop
>> the whole nullglob bit and just do something like:
>>
>> contents=*
>> if [[ $contents = "*" ]]; then

martti@deepthought:~/test$ touch *
martti@deepthought:~/test$ contents=(*)
martti@deepthought:~/test$ echo "${contents[*]}"
*
martti@deepthought:~/test$ echo ${#contents}
1

Don't approve. Dave's version fails if there's a valid file called
'*', as little as it's probable.
Better do bash ugly and safe.
 
Old 06-24-2011, 01:43 PM
Martti Kühne
 
Default repo-add: avoid using ls to check for presence of files

On Fri, Jun 24, 2011 at 6:34 AM, Dave Reisner <d@falconindy.com> wrote:
> On Thu, Jun 23, 2011 at 11:06:45PM -0500, Dan McGee wrote:
>> On Wed, Jun 22, 2011 at 7:38 PM, Dave Reisner <d@falconindy.com> wrote:
>>
>> I agree with the commit message, but damn is this ugly. Can't we drop
>> the whole nullglob bit and just do something like:
>>
>> contents=*
>> if [[ $contents = "*" ]]; then

martti@deepthought:~/test$ touch *
martti@deepthought:~/test$ contents=(*)
martti@deepthought:~/test$ echo "${contents[*]}"
*
martti@deepthought:~/test$ echo ${#contents}
1

Don't approve. Dave's version fails if there's a valid file called
'*', as little as it's probable.
Better do bash ugly and safe.
 
Old 06-24-2011, 04:52 PM
Dan McGee
 
Default repo-add: avoid using ls to check for presence of files

On Wed, Jun 22, 2011 at 7:38 PM, Dave Reisner <d@falconindy.com> wrote:
> Signed-off-by: Dave Reisner <d@falconindy.com>
> ---
> *scripts/repo-add.sh.in | * *2 +-
> *1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> index 029e17d..e75055f 100644
> --- a/scripts/repo-add.sh.in
> +++ b/scripts/repo-add.sh.in
> @@ -593,7 +593,7 @@ if (( success )); then
> * * * *filename=${REPO_DB_FILE##*/}
>
> * * * *pushd "$tmpdir" >/dev/null
> - * * * if [[ -n $(ls) ]]; then
> + * * * if ( shopt -s nullglob; files=(*); (( ${#files[*]} )) ); then
> * * * * * * * *bsdtar -c${TAR_OPT}f "$filename" *
> * * * *else
> * * * * * * * *# we have no packages remaining? zip up some emptyness

Let's take a step back and just kill the entire conditional:

bsdtar -c${TAR_OPT}f "$filename" -s /^.// .

"Add the current directory, but we don't want the ./ bits on the added paths."
 

Thread Tools




All times are GMT. The time now is 07:56 AM.

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