[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing t
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary |
Date: |
Wed, 25 Oct 2017 22:11:29 +0100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Wed, Oct 25, 2017 at 07:10:40PM +1100, Alexey Kardashevskiy wrote:
> On 25/10/17 17:57, Daniel P. Berrange wrote:
> > On Wed, Oct 25, 2017 at 12:45:10PM +1100, Alexey Kardashevskiy wrote:
> >> On 25/10/17 03:27, Daniel P. Berrange wrote:
> >>> On Tue, Oct 24, 2017 at 07:58:53PM +1100, Alexey Kardashevskiy wrote:
> >>>> The new git-submodule.sh script writes .git-submodule-status to
> >>>> the source directory every time no matter what. This makes it
> >>>> conditional.
> >>>>
> >>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> >>>> ---
> >>>>
> >>>> I compile out of tree on a remote guest system where I mount the
> >>>> source directory as "readonly" and build directory as "rw" and
> >>>> scripts/git-submodule.sh tries writing to the source directory even when
> >>>> I manually update modules on a host machine which is quite annoying.
> >>>>
> >>>> Is this something acceptable? Or I am missing something here?
> >>>
> >>> How did you update the modules - did you manually run 'git submodule
> >>> update...'
> >>> or did you use the git-submodule.sh script on your host machine ?
> >>
> >>
> >> I run scripts/git-submodule.sh. Which is not thrilling either as I rather
> >> expect source tree not to be affected in any way when running "make".
> >
> > Oh, did you pass the list of sub-modules to it when running
> >
> > eg, ./scripts/git-submodule.sh update ui/keycodemapdb
> >
> > the list of submodules you need is printed in the configure output summary.
>
> Sure, otherwise it does nothing.
>
>
> >
> >>> If you run git-submodule.sh on the host, then it should save the status
> >>> file, and then when you run make on the guest system, it should notice
> >>> that you're already updated and never even invoke 'git-submodule.sh
> >>> update'
> >>
> >>
> >> scripts/git-submodule.sh also tries writing to the source directory (I
> >> should probably have fixed that branch too) but this failure is not fatal
> >> for "make" but makes it want to try "update" and then "make" fails.
> >
> > This shouldn't have happened in your case though, if you have already run
> > 'git-submodule.sh update ...list of modules...' on the host machine, with
> > the same list of modules that the guest 'configure' printed out.
>
> It does not matter if I run git-submodule.sh or not - "git-submodule.sh
> status" will try writing to the read only folder anyway and it will fail
> and Makefile's git_module_status will be set to 1.
Ahhhh, great, now I understand why you're hitting the problem !
> If I do as below (and that's what I should have done as I said), then
> "git-submodule.sh update" is not invoked and we are good. I am not
> reposting it yet as 1) my shell skills are crap (need to delete the temp
> file or rewrite the whole thing not to use temp file or rewrite it in
> python - why do not people use python everywhere?!) 2) I still hope we stop
> doing this from Makefile :)
I agree using a tmpfile is the right fix here.
>
>
>
> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> index d8fbc7e47e..ced00b6cae 100755
> --- a/scripts/git-submodule.sh
> +++ b/scripts/git-submodule.sh
> @@ -23,16 +23,20 @@ then
> exit 1
> fi
>
> +substat_tmp=$(mktemp)
> +
> case "$command" in
> status)
> test -f "$substat" || exit 1
> - trap "rm -f ${substat}.tmp" EXIT
> - git submodule status $modules > "${substat}.tmp"
> - diff "${substat}" "${substat}.tmp" >/dev/null
> + git submodule status $modules > "$substat_tmp"
> + diff "${substat_tmp}" "${substat}" || mv "${substat_tmp}" "${substat}"
We shouldn't be overwriting the existing status file during 'status' - only
'update' should overwrite. 'status' should be a no-op query only.
> + ( rm "${substat_tmp}" 2>/dev/null )
> exit $?
> ;;
> update)
> git submodule update --init $modules 1>/dev/null 2>&1
> - git submodule status $modules > "${substat}"
> + git submodule status $modules > "$substat_tmp"
> + diff "${substat_tmp}" "${substat}" || mv "${substat_tmp}" "${substat}"
> + ( rm "${substat_tmp}" 2>/dev/null )
> ;;
> esac
>
>
>
> --
> Alexey
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Alexey Kardashevskiy, 2017/10/24
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Daniel P. Berrange, 2017/10/24
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Peter Maydell, 2017/10/24
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Alexey Kardashevskiy, 2017/10/24
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Daniel P. Berrange, 2017/10/25
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Alexey Kardashevskiy, 2017/10/25
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary,
Daniel P. Berrange <=
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Alexey Kardashevskiy, 2017/10/25
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Daniel P. Berrange, 2017/10/26
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Alexey Kardashevskiy, 2017/10/26
- Re: [Qemu-devel] [RFC PATCH qemu] git-submodule.sh: Do not try writing to source directory if not necessary, Peter Maydell, 2017/10/26