[vlc-devel] [PATCH] contrib: remove empty file on download fail

Thomas Guillem thomas at gllm.fr
Wed Aug 26 13:58:16 CEST 2020



On Wed, Aug 26, 2020, at 11:40, Alexandre Janniaux wrote:
> Hi,
> 
> On Wed, Aug 26, 2020 at 11:32:25AM +0200, Thomas Guillem wrote:
> >
> >
> > On Wed, Aug 26, 2020, at 11:22, Alexandre Janniaux wrote:
> > > Hi,
> > >
> > > This is probably missing `-f` in order to avoid failure with
> > > the rm command, and the rm should probably be done before curl
> > > so as to remove the dependency if it exists and correctly return
> > > an error in case of failure with curl.
> >
> > If it's done before the curl command then download won't be executed again after a failure. That is what I'm trying to solve. (makefile dependencies)
> 
> Then a cleaner solution would be to download the file at a
> temporary (unique per file) location and move it to the correct
> location after it finished.

OK, sent a new version. Thanks for the review !

> 
> > Also, the download command is not appending but overriding, this make a rm before curl useless.
> 
> The rm at the beginning would be to be sure the command is
> stateless, but in the general yes, rm might be useless.
> I don't know curl enough to know the failure case so you might
> be right.
> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> >
> >
> > >
> > >    download = rm -f "$@" && curl -f -L -- "$(1)" > "$@"
> > >
> > > Regards,
> > > --
> > > Alexandre Janniaux
> > > Videolabs
> > >
> > > On Wed, Aug 26, 2020 at 11:09:26AM +0200, Thomas Guillem wrote:
> > > > This fixes package not downloaded again in case of a first failure.
> > > >
> > > > The other download rules (wget, fetch) are already following this behavior on
> > > > fail.
> > > > ---
> > > >  contrib/src/main.mak | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/contrib/src/main.mak b/contrib/src/main.mak
> > > > index d0df860a954..fc1480cca19 100644
> > > > --- a/contrib/src/main.mak
> > > > +++ b/contrib/src/main.mak
> > > > @@ -247,7 +247,7 @@ endif
> > > >  SVN ?= $(error subversion client (svn) not found)
> > > >
> > > >  ifeq ($(shell curl --version >/dev/null 2>&1 || echo FAIL),)
> > > > -download = curl -f -L -- "$(1)" > "$@"
> > > > +download = curl -f -L -- "$(1)" > "$@" || rm "$@"
> > > >  else ifeq ($(shell wget --version >/dev/null 2>&1 || echo FAIL),)
> > > >  download = (rm -f $@.tmp && \
> > > >  	wget --passive -c -p -O $@.tmp "$(1)" && \
> > > > --
> > > > 2.28.0
> > > >
> > > > _______________________________________________
> > > > vlc-devel mailing list
> > > > To unsubscribe or modify your subscription options:
> > > > https://mailman.videolan.org/listinfo/vlc-devel
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list