| Submitter | Robert Yang |
|---|---|
| Date | Sept. 5, 2012, 9:31 a.m. |
| Message ID | <a30f2ce625990ccfb3e1e06794b353dc1f1257cb.1346837268.git.liezhi.yang@windriver.com> |
| Download | mbox | patch |
| Permalink | /patch/35919/ |
| State | New |
| Headers | show |
Comments
Op 5 sep. 2012, om 11:31 heeft Robert Yang <liezhi.yang@windriver.com> het volgende geschreven: > This is for fixing the problem: > 1) bitbake core-image-sato-sdk with MACHINE=qemux86 > 2) bitbake core-image-sato with with MACHINE=crownbay > > The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several > i586 packages will be installed into crownbay's rootfs though there are > core2 packages. For example, there are: > > xserver-xorg_1.11.2-r4_i586.ipk > xserver-xorg_1.9.3-r1_core2.ipk > > The crownbay.conf says: > PREFERRED_VERSION_xserver-xorg ?= "1.9.3" > > What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but > the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is > incorrect. It is the correct behaviour, though. > This is caused by opkg's selecting mechanism: when more than one > candidate is found, it will use the higher version one and ignore the > arch priority. As expected and wanted. > we have several conf files which set the PREFERRED_VERSION_pkg = "..." , > but there is no such a mechanism which can let us tell the opkg to > install the preferred version. As pointed out before, your DISTRO needs to fix this and you should NOT break opkg in the way you are doing now.
On Wed, 2012-09-05 at 12:05 +0200, Koen Kooi wrote: > Op 5 sep. 2012, om 11:31 heeft Robert Yang <liezhi.yang@windriver.com> het volgende geschreven: > > > This is for fixing the problem: > > 1) bitbake core-image-sato-sdk with MACHINE=qemux86 > > 2) bitbake core-image-sato with with MACHINE=crownbay > > > > The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several > > i586 packages will be installed into crownbay's rootfs though there are > > core2 packages. For example, there are: > > > > xserver-xorg_1.11.2-r4_i586.ipk > > xserver-xorg_1.9.3-r1_core2.ipk > > > > The crownbay.conf says: > > PREFERRED_VERSION_xserver-xorg ?= "1.9.3" > > > > What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but > > the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is > > incorrect. > > It is the correct behaviour, though. No, it isn't. You told bitbake to build some specific versions, it did that, then put something else in the rootfs. Without mentioning any of this to the user. So the current behaviour is totally broken and it needs to do one of: a) Put the things the user asked for in the rootfs b) Tell the user its not going to c) Error out and ask the user to fix the problem Builds are meant to be deterministic and this clearly isn't as what you get out depends on the order of what you build. > > This is caused by opkg's selecting mechanism: when more than one > > candidate is found, it will use the higher version one and ignore the > > arch priority. > > As expected and wanted. Under certain situations but not this one. > > we have several conf files which set the PREFERRED_VERSION_pkg = "..." , > > but there is no such a mechanism which can let us tell the opkg to > > install the preferred version. > > As pointed out before, your DISTRO needs to fix this and you should > NOT break opkg in the way you are doing now. Maybe. What you're suggesting is that we have to force every package on a given architecture to the lowest common denominator which makes no sense. I understand about the "un-removable" package issue you previously mentioned. There is a difference between what we build at rootfs time and any ultimate disto package feeds however and I think we need to at least consider this proposal. It would appear at face value to at least make builds deterministic and do what the users expects them to do. The side effect is making on target package management more difficult. Going back to the un-removable package problem, I think that opkg probably should always prefer arch specific over version. The question is how do you get back to say arch specific rather than machine specific at some later date. I'd argue that removing the arch specific package from the feed would be the trigger for that. Could that work as a mechanism to get us out of this problematic situation? Please try and help us figure out a way out of this. I will warn that above all else builds need to be deterministic and so Robert's patch is tempting right now as it fixes that. Cheers, Richard
Op 5 sep. 2012, om 13:44 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven: > On Wed, 2012-09-05 at 12:05 +0200, Koen Kooi wrote: >> Op 5 sep. 2012, om 11:31 heeft Robert Yang <liezhi.yang@windriver.com> het volgende geschreven: >> >>> This is for fixing the problem: >>> 1) bitbake core-image-sato-sdk with MACHINE=qemux86 >>> 2) bitbake core-image-sato with with MACHINE=crownbay >>> >>> The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several >>> i586 packages will be installed into crownbay's rootfs though there are >>> core2 packages. For example, there are: >>> >>> xserver-xorg_1.11.2-r4_i586.ipk >>> xserver-xorg_1.9.3-r1_core2.ipk >>> >>> The crownbay.conf says: >>> PREFERRED_VERSION_xserver-xorg ?= "1.9.3" >>> >>> What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but >>> the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is >>> incorrect. >> >> It is the correct behaviour, though. > > No, it isn't. You told bitbake to build some specific versions, it did > that, then put something else in the rootfs. Without mentioning any of > this to the user. You forget that, you, as the user, instructed bitbake to build the other version when switching machine. Should bitbake refuse to build the packages in this scenario? That would make more sense than trying to clean up the mess in the package_ipk bbclass. If you have online feeds the problem will reappear anyway. > So the current behaviour is totally broken and it needs to do one of: > > a) Put the things the user asked for in the rootfs > b) Tell the user its not going to > c) Error out and ask the user to fix the problem > > Builds are meant to be deterministic and this clearly isn't as what you > get out depends on the order of what you build. How do we know what the user expects? The user already did something that isn't right by building compatible arch packages with different versions. And this is deterministic, the user instructed bitbake to build a more recent, but compatible version, which will end up in the rootfs. If would be non-deterministic if opkg would decide at random what to install. So, fix this problem at the root and make bitbake error out if your build breaks PREFERRED_VERSION.
2012/9/5 Koen Kooi <koen@dominion.thruhere.net>: > > Op 5 sep. 2012, om 13:44 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven: > >> On Wed, 2012-09-05 at 12:05 +0200, Koen Kooi wrote: >>> Op 5 sep. 2012, om 11:31 heeft Robert Yang <liezhi.yang@windriver.com> het volgende geschreven: >>> >>>> This is for fixing the problem: >>>> 1) bitbake core-image-sato-sdk with MACHINE=qemux86 >>>> 2) bitbake core-image-sato with with MACHINE=crownbay >>>> >>>> The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several >>>> i586 packages will be installed into crownbay's rootfs though there are >>>> core2 packages. For example, there are: >>>> >>>> xserver-xorg_1.11.2-r4_i586.ipk >>>> xserver-xorg_1.9.3-r1_core2.ipk >>>> >>>> The crownbay.conf says: >>>> PREFERRED_VERSION_xserver-xorg ?= "1.9.3" >>>> >>>> What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but >>>> the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is >>>> incorrect. >>> >>> It is the correct behaviour, though. >> >> No, it isn't. You told bitbake to build some specific versions, it did >> that, then put something else in the rootfs. Without mentioning any of >> this to the user. > > You forget that, you, as the user, instructed bitbake to build the other version when switching machine. Should bitbake refuse to build the packages in this scenario? That would make more sense than trying to clean up the mess in the package_ipk bbclass. If you have online feeds the problem will reappear anyway. > >> So the current behaviour is totally broken and it needs to do one of: >> >> a) Put the things the user asked for in the rootfs >> b) Tell the user its not going to >> c) Error out and ask the user to fix the problem >> >> Builds are meant to be deterministic and this clearly isn't as what you >> get out depends on the order of what you build. > > How do we know what the user expects? The user already did something that isn't right by building compatible arch packages with different versions. And this is deterministic, the user instructed bitbake to build a more recent, but compatible version, which will end up in the rootfs. If would be non-deterministic if opkg would decide at random what to install. > > So, fix this problem at the root and make bitbake error out if your build breaks PREFERRED_VERSION. +1 Even thought that this won't hit me again with a dedicated build machine that deletes the tmp/sstate dirs before each build. I can remember building for the Pandaboard and Overo in a row which resulted in errors that could not be resolved with sstate clean, leading to unexpected runtime behavior but "stated" a completely sane build. It turned out that the PREFERRED_VERSION for an Overo recipe was ignored, deleting the tmp/sstate directories did sort out the problem.
On Wed, 2012-09-05 at 15:24 +0200, Koen Kooi wrote: > Op 5 sep. 2012, om 13:44 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven: > > > On Wed, 2012-09-05 at 12:05 +0200, Koen Kooi wrote: > >> Op 5 sep. 2012, om 11:31 heeft Robert Yang <liezhi.yang@windriver.com> het volgende geschreven: > >> > >>> This is for fixing the problem: > >>> 1) bitbake core-image-sato-sdk with MACHINE=qemux86 > >>> 2) bitbake core-image-sato with with MACHINE=crownbay > >>> > >>> The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several > >>> i586 packages will be installed into crownbay's rootfs though there are > >>> core2 packages. For example, there are: > >>> > >>> xserver-xorg_1.11.2-r4_i586.ipk > >>> xserver-xorg_1.9.3-r1_core2.ipk > >>> > >>> The crownbay.conf says: > >>> PREFERRED_VERSION_xserver-xorg ?= "1.9.3" > >>> > >>> What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but > >>> the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is > >>> incorrect. > >> > >> It is the correct behaviour, though. > > > > No, it isn't. You told bitbake to build some specific versions, it did > > that, then put something else in the rootfs. Without mentioning any of > > this to the user. > > You forget that, you, as the user, instructed bitbake to build the > other version when switching machine. Should bitbake refuse to build > the packages in this scenario? That would be better than silently doing something non-deterministic. The bit I hate about this is the fact that sometimes a build would result in one thing, sometimes another. It should always error out with an invalid configuration rather than do that. > That would make more sense than trying to clean up the mess in the > package_ipk bbclass. If you have online feeds the problem will > reappear anyway. I care more about builds being deterministic than online feeds. Sorry ;-). > > So the current behaviour is totally broken and it needs to do one of: > > > > a) Put the things the user asked for in the rootfs > > b) Tell the user its not going to > > c) Error out and ask the user to fix the problem > > > > Builds are meant to be deterministic and this clearly isn't as what you > > get out depends on the order of what you build. > > How do we know what the user expects? The user already did something > that isn't right by building compatible arch packages with different > versions. And this is deterministic, the user instructed bitbake to > build a more recent, but compatible version, which will end up in the > rootfs. If would be non-deterministic if opkg would decide at random > what to install. Having a different image depending on whether I build MACHINE A or B first is not what the user expects and is not deterministic in my world or I suspect in most user's. We can go and ask some if you really think we need to? > So, fix this problem at the root and make bitbake error out if your > build breaks PREFERRED_VERSION. How do we detect this? I want deterministic builds so I need the error to appear regardless of whether I build A or B first (just like I expect a consistent image). I also do want to support these situations where we need special versions. I appreciate its ugly but we have several significant users with this problem and pretending it doesn't exist doesn't work, much as I wish we could. What I really need here is help with coming up with some working solution. Putting our heads in the sand and arguing whether its even a problem isn't going to go anywhere :(. Its really easy to shoot down ideas on the mailing list. Its much harder to be creative and find ways to take the project to better places whilst addressing everyone's concerns. I'm starting to find I'm simply physically and mentally running out of bandwidth for some of these discussions. I try very hard to hear different opinions, explain decisions, come up with creative solutions and so on and I think this process is one of the better features of the project. I am going to need help in order to be able to continue to do this and scale the project though. Cheers, Richard
On Wed, Sep 5, 2012 at 2:19 PM, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > What I really need here is help with coming up with some working > solution. Putting our heads in the sand and arguing whether its even a > problem isn't going to go anywhere :(. This doesn't make any sense. Deciding whether something is or isn't an actual problem / is or isn't expected behavior is a good thing to be doing, and is the opposite of putting heads in the sand, as it's debating user expectations vs tool behavior. If you really think it's better to blindly make changes before figuring out how the tool should behave relative to user expectations, or think figuring out what should be done first is "putting our heads in the sand", then I think we have serious problems with you in charge of the project.
On Wed, 2012-09-05 at 15:19 -0700, Chris Larson wrote: > On Wed, Sep 5, 2012 at 2:19 PM, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > What I really need here is help with coming up with some working > > solution. Putting our heads in the sand and arguing whether its even a > > problem isn't going to go anywhere :(. > > This doesn't make any sense. Deciding whether something is or isn't an > actual problem / is or isn't expected behavior is a good thing to be > doing, and is the opposite of putting heads in the sand, as it's > debating user expectations vs tool behavior. If you really think it's > better to blindly make changes before figuring out how the tool should > behave relative to user expectations, or think figuring out what > should be done first is "putting our heads in the sand", then This isn't what I was suggesting and is misinterpretation. Given the following comment I'm not sure its worth taking the discuss further. > I think > we have serious problems with you in charge of the project. Your opinion is duly noted, thanks ;-). Cheers, Richard
Op 5 sep. 2012, om 23:19 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven: > On Wed, 2012-09-05 at 15:24 +0200, Koen Kooi wrote: >> Op 5 sep. 2012, om 13:44 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven: >> >>> On Wed, 2012-09-05 at 12:05 +0200, Koen Kooi wrote: >>>> Op 5 sep. 2012, om 11:31 heeft Robert Yang <liezhi.yang@windriver.com> het volgende geschreven: >>>> >>>>> This is for fixing the problem: >>>>> 1) bitbake core-image-sato-sdk with MACHINE=qemux86 >>>>> 2) bitbake core-image-sato with with MACHINE=crownbay >>>>> >>>>> The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several >>>>> i586 packages will be installed into crownbay's rootfs though there are >>>>> core2 packages. For example, there are: >>>>> >>>>> xserver-xorg_1.11.2-r4_i586.ipk >>>>> xserver-xorg_1.9.3-r1_core2.ipk >>>>> >>>>> The crownbay.conf says: >>>>> PREFERRED_VERSION_xserver-xorg ?= "1.9.3" >>>>> >>>>> What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but >>>>> the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is >>>>> incorrect. >>>> >>>> It is the correct behaviour, though. >>> >>> No, it isn't. You told bitbake to build some specific versions, it did >>> that, then put something else in the rootfs. Without mentioning any of >>> this to the user. >> >> You forget that, you, as the user, instructed bitbake to build the >> other version when switching machine. Should bitbake refuse to build >> the packages in this scenario? > > That would be better than silently doing something non-deterministic. > > The bit I hate about this is the fact that sometimes a build would > result in one thing, sometimes another. It should always error out with > an invalid configuration rather than do that. > >> That would make more sense than trying to clean up the mess in the >> package_ipk bbclass. If you have online feeds the problem will >> reappear anyway. > > I care more about builds being deterministic than online feeds. Thanks for going on the record for that. > Sorry ;-). > >>> So the current behaviour is totally broken and it needs to do one of: >>> >>> a) Put the things the user asked for in the rootfs >>> b) Tell the user its not going to >>> c) Error out and ask the user to fix the problem >>> >>> Builds are meant to be deterministic and this clearly isn't as what you >>> get out depends on the order of what you build. >> >> How do we know what the user expects? The user already did something >> that isn't right by building compatible arch packages with different >> versions. And this is deterministic, the user instructed bitbake to >> build a more recent, but compatible version, which will end up in the >> rootfs. If would be non-deterministic if opkg would decide at random >> what to install. > > Having a different image depending on whether I build MACHINE A or B > first is not what the user expects and is not deterministic in my world > or I suspect in most user's. We can go and ask some if you really think > we need to? > >> So, fix this problem at the root and make bitbake error out if your >> build breaks PREFERRED_VERSION. > > How do we detect this? I want deterministic builds so I need the error > to appear regardless of whether I build A or B first (just like I expect > a consistent image). > > I also do want to support these situations where we need special > versions. I appreciate its ugly but we have several significant users > with this problem and pretending it doesn't exist doesn't work, much as > I wish we could. > > What I really need here is help with coming up with some working > solution. Putting our heads in the sand and arguing whether its even a > problem isn't going to go anywhere :(. Where have I argued that it's not a problem? I've said time and time again that PREFERRED_VERSION problems are for the DISTRO to solve. You are arguing that we should band-aid it somewhere in a class where it would break package feeds. So my counter proposal for broken DISTROs was to have bitbake completely prevent the user from building such a conflicting version. The low-tech solution would be to document that you can't build this combination of machines in the same TMPDIR. Wiping TMDIR between machines changes should be quite cheap with sstate-cache nowadays, no?
On Thu, 2012-09-06 at 13:05 +0200, Koen Kooi wrote: > Op 5 sep. 2012, om 23:19 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven: > > > On Wed, 2012-09-05 at 15:24 +0200, Koen Kooi wrote: > >> Op 5 sep. 2012, om 13:44 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven: > >> > >>> On Wed, 2012-09-05 at 12:05 +0200, Koen Kooi wrote: > >>>> Op 5 sep. 2012, om 11:31 heeft Robert Yang <liezhi.yang@windriver.com> het volgende geschreven: > >>>> > >>>>> This is for fixing the problem: > >>>>> 1) bitbake core-image-sato-sdk with MACHINE=qemux86 > >>>>> 2) bitbake core-image-sato with with MACHINE=crownbay > >>>>> > >>>>> The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several > >>>>> i586 packages will be installed into crownbay's rootfs though there are > >>>>> core2 packages. For example, there are: > >>>>> > >>>>> xserver-xorg_1.11.2-r4_i586.ipk > >>>>> xserver-xorg_1.9.3-r1_core2.ipk > >>>>> > >>>>> The crownbay.conf says: > >>>>> PREFERRED_VERSION_xserver-xorg ?= "1.9.3" > >>>>> > >>>>> What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but > >>>>> the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is > >>>>> incorrect. > >>>> > >>>> It is the correct behaviour, though. > >>> > >>> No, it isn't. You told bitbake to build some specific versions, it did > >>> that, then put something else in the rootfs. Without mentioning any of > >>> this to the user. > >> > >> You forget that, you, as the user, instructed bitbake to build the > >> other version when switching machine. Should bitbake refuse to build > >> the packages in this scenario? > > > > That would be better than silently doing something non-deterministic. > > > > The bit I hate about this is the fact that sometimes a build would > > result in one thing, sometimes another. It should always error out with > > an invalid configuration rather than do that. > > > >> That would make more sense than trying to clean up the mess in the > >> package_ipk bbclass. If you have online feeds the problem will > >> reappear anyway. > > > > I care more about builds being deterministic than online feeds. > > Thanks for going on the record for that. We're a build system and package feed creation has always been argued to be a second step based on the output of the build system, often by you iirc. > > > >> So, fix this problem at the root and make bitbake error out if your > >> build breaks PREFERRED_VERSION. > > > > How do we detect this? I want deterministic builds so I need the error > > to appear regardless of whether I build A or B first (just like I expect > > a consistent image). > > > > I also do want to support these situations where we need special > > versions. I appreciate its ugly but we have several significant users > > with this problem and pretending it doesn't exist doesn't work, much as > > I wish we could. > > > > What I really need here is help with coming up with some working > > solution. Putting our heads in the sand and arguing whether its even a > > problem isn't going to go anywhere :(. > > Where have I argued that it's not a problem? I've said time and time > again that PREFERRED_VERSION problems are for the DISTRO to solve. You > are arguing that we should band-aid it somewhere in a class where it > would break package feeds. So my counter proposal for broken DISTROs > was to have bitbake completely prevent the user from building such a > conflicting version. I'd argue its up to the DISTRO to solve at package feed generation time and that is the point it should error. I cannot see any way bitbake can reasonably figure out this problem and accurately error as it simply doesn't have the info. At package feed generation time, you do have that info. On the other hand, the build system *can* generate images consistently. > The low-tech solution would be to document that you can't build this > combination of machines in the same TMPDIR. Wiping TMDIR between > machines changes should be quite cheap with sstate-cache nowadays, no? By your own reasoning, this is unacceptable since the generated images would differ from what online package management would result in with image upgrades. Documenting the builds aren't deterministic is a fail, we can do better. Cheers, Richard
Patch
diff --git a/meta/classes/package_ipk.bbclass b/meta/classes/package_ipk.bbclass index e94586e..c274296 100644 --- a/meta/classes/package_ipk.bbclass +++ b/meta/classes/package_ipk.bbclass @@ -71,7 +71,8 @@ package_tryout_install_multilib_ipk() { multilib_tryout_dirs="" for item in ${MULTILIB_VARIANTS}; do local target_rootfs="${MULTILIB_TEMP_ROOTFS}/${item}" - local ipkg_args="-f ${INSTALL_CONF_IPK} -o ${target_rootfs} --force_overwrite" + local ipkg_args="-f ${INSTALL_CONF_IPK} -o ${target_rootfs} \ + --force_overwrite --force-arch" local selected_pkg="" local pkgname_prefix="${item}-" local pkgname_len=${#pkgname_prefix} @@ -139,7 +140,8 @@ package_install_internal_ipk() { mkdir -p ${target_rootfs}${localstatedir}/lib/opkg/ - local ipkg_args="-f ${conffile} -o ${target_rootfs} --force-overwrite --force_postinstall" + local ipkg_args="-f ${conffile} -o ${target_rootfs} --force-overwrite \ + --force_postinstall --force-arch" opkg-cl ${ipkg_args} update
This is for fixing the problem: 1) bitbake core-image-sato-sdk with MACHINE=qemux86 2) bitbake core-image-sato with with MACHINE=crownbay The qemux86's PACKAGE_ARCH is i586, the crownbay's is core2, but several i586 packages will be installed into crownbay's rootfs though there are core2 packages. For example, there are: xserver-xorg_1.11.2-r4_i586.ipk xserver-xorg_1.9.3-r1_core2.ipk The crownbay.conf says: PREFERRED_VERSION_xserver-xorg ?= "1.9.3" What the crownbay's image needs is xserver-xorg_1.9.3-r1_core2.ipk, but the xserver-xorg_1.11.2-r4_i586.ipk will be installed, this is incorrect. This is caused by opkg's selecting mechanism: when more than one candidate is found, it will use the higher version one and ignore the arch priority. we have several conf files which set the PREFERRED_VERSION_pkg = "..." , but there is no such a mechanism which can let us tell the opkg to install the preferred version. When the preferred version is higher, this is OK, but if the preferred version is lower, there would be problems: 1) Most of the packages are core2 in the image, but several of them are i586, though we have built the core2 ones, this seems strange. 2) What's worse is that the image may not work since the preferred version pkg is not installed. We have set the arch priority clearly in the opkg.conf, I think that respect to the arch priority is reasonable during the image generation, use the "--force-arch" would fix the problem. Signed-off-by: Robert Yang <liezhi.yang@windriver.com> --- meta/classes/package_ipk.bbclass | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)