From: Samuli Suominen <ssuominen@gentoo.org>
To: NeilBrown <neilb@suse.de>
Cc: linux-raid@vger.kernel.org
Subject: Re: [PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services)
Date: Fri, 18 Jul 2014 16:51:43 +0300 [thread overview]
Message-ID: <53C9266F.40300@gentoo.org> (raw)
In-Reply-To: <53C789E1.50604@gentoo.org>
This new patch, that only changes the filename, was tested and verified
to fix the problem in the downstream Gentoo bug,
https://bugs.gentoo.org/show_bug.cgi?id=517218#c18
So you were right :)
On 17/07/14 11:31, Samuli Suominen wrote:
> On 17/07/14 11:25, NeilBrown wrote:
>> On Thu, 17 Jul 2014 10:30:05 +0300 Samuli Suominen <ssuominen@gentoo.org>
>> wrote:
>>
>>> If you run with:
>>>
>>> $ export MAKEFLAGS="-j9"
>>> $ make install install-systemd
>>>
>>> As in, combine "install" and "install-systemd" in the same command using
>>> parallel make,
>>> the content of .service files could end up in .rules, or otherway around
>>> because GNU
>>> make fill run them in parallel and the commands might ran at the same
>>> time, and both
>>> use same variables $file and same temporary files .install.tmp
>>>
>>> This was reported at,
>>> https://bugs.gentoo.org/show_bug.cgi?id=517218
>>>
>>> The immediate workaround is,
>>>
>>> $ export MAKEFLAGS="-j9"
>>> $ make install
>>> $ make install-systemd
>>>
>>> And the attach patch makes it full proof so they can be ran at the same
>>> line.
>>>
>>> Thanks,
>>> Samuli
>> Hi,
>> thanks for the bug report and the patch!
>>
>> I don't think it is necessary to change the name for the variable (file ->
>> file1, file2 or file3). Just leave it as 'file'.
>> However changing the name is important. Could you send a patch which just
>> does that, and I'll apply it.
>>
>> Also if the patch could be included inline in the mail, rather than as an
>> attachment, that would be nice.
>>
>> (But if you don't want to bother, I'll make the fix anyway and credit you).
>>
>> Thanks,
>> NeilBrown
> Thanks for the review, I wasn't 100% sure which I should be changing,
> the variable, or the filename,
> so I changed both.
> So, you are most likely correct.
>
> Here is the patch that only changes the filename,
>
> --- mdadm-3.3.1.orig/Makefile
> +++ mdadm-3.3.1/Makefile
> @@ -282,25 +282,25 @@
>
> install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
> @for file in 63-md-raid-arrays.rules 64-md-raid-assembly.rules ; \
> - do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp && \
> + do sed -e 's,BINDIR,$(BINDIR),g' udev-$${file#??-} > .install.tmp.1
> && \
> echo $(INSTALL) -D -m 644 udev-$${file#??-}
> $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> - $(INSTALL) -D -m 644 .install.tmp
> $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> - rm -f .install.tmp; \
> + $(INSTALL) -D -m 644 .install.tmp.1
> $(DESTDIR)$(UDEVDIR)/rules.d/$$file ; \
> + rm -f .install.tmp.1; \
> done
>
> install-systemd: systemd/mdmon@.service
> @for file in mdmon@.service mdmonitor.service
> mdadm-last-resort@.timer \
> mdadm-last-resort@.service ; \
> - do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
> + do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp.2 && \
> echo $(INSTALL) -D -m 644 systemd/$$file
> $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
> - $(INSTALL) -D -m 644 .install.tmp
> $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
> - rm -f .install.tmp; \
> + $(INSTALL) -D -m 644 .install.tmp.2
> $(DESTDIR)$(SYSTEMD_DIR)/$$file ; \
> + rm -f .install.tmp.2; \
> done
> @for file in mdadm.shutdown ; \
> - do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp && \
> + do sed -e 's,BINDIR,$(BINDIR),g' systemd/$$file > .install.tmp.3 && \
> echo $(INSTALL) -D -m 755 systemd/$$file
> $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
> - $(INSTALL) -D -m 755 .install.tmp
> $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
> - rm -f .install.tmp; \
> + $(INSTALL) -D -m 755 .install.tmp.3
> $(DESTDIR)$(SYSTEMD_DIR)-shutdown/$$file ; \
> + rm -f .install.tmp.3; \
> done
> if [ -f /etc/SuSE-release -o -n "$(SUSE)" ] ;then $(INSTALL) -D -m
> 755 systemd/SUSE-mdadm_env.sh
> $(DESTDIR)$(SYSTEMD_DIR)/../scripts/mdadm_env.sh ;fi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
prev parent reply other threads:[~2014-07-18 13:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-17 7:30 [PATCH] Parallel make bug in Makefile of mdadm-3.3.1 (corrupted udev rules and/or systemd services) Samuli Suominen
2014-07-17 7:45 ` Samuli Suominen
2014-07-17 8:25 ` NeilBrown
2014-07-17 8:31 ` Samuli Suominen
2014-07-18 13:51 ` Samuli Suominen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53C9266F.40300@gentoo.org \
--to=ssuominen@gentoo.org \
--cc=linux-raid@vger.kernel.org \
--cc=neilb@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.