| Message ID | 20180730155153.24091-4-m.patzlaff@pilz.de |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Buildroot: Target to update config fragments in kconfig based packages | expand |
Hello Marcel, On Mon, 30 Jul 2018 17:51:53 +0200, Marcel Patzlaff wrote: > This patch adds the new target above which implements the update routine as > detailled in the head of this patch series. > > Signed-off-by: Marcel Patzlaff <m.patzlaff@pilz.de> Thanks for working on this complex topic, as Arnout said. On my side, I find the semantic of "let's update the _last_ fragment" to be a bit weird and complex to understand. Why the last one, and not any other ? I'm not sure it's possible to have something that updates fragments with a sensible semantic. However, what would be useful would be a way to dump the difference between the current configuration, and what the combination of the defconfig+fragments provide. I.e, you start Buildroot with omap2plus_defconfig + some specific fragments as the kernel configuration. You run "make linux-show-config-diff", which returns empty. Then you tweak the configuration with "make linux-menuconfig", and run "make linux-show-config-diff", which shows you the lines that you need to put in one of your fragments to preserve the configuration tweaks you have done. Of course, the name "linux-show-config-diff" is just made up here, and some more thought is needed to find a good name. But overall, I believe the semantic would be much clearer, and doesn't make any assumption on whether we have one or several fragments, and whether the last fragment or any other fragment needs to be updated. What do you think about this? Would this also fill your needs? Thanks! Thomas
Hello Thomas, > Hello Marcel, > > Thanks for working on this complex topic, as Arnout said. > > On my side, I find the semantic of "let's update the _last_ fragment" > to be a bit weird and complex to understand. Why the last one, and not > any other ? > > I'm not sure it's possible to have something that updates fragments > with a sensible semantic. Well, it should just reflect what the merge_config script does: the last file in the list of files to be merged always overrides settings done in files before. So this means, that the last fragment is basically the most significate one. When I think about it, this should also be documented somewhere. Because the result with configuration fragments A, B, C could be different than B, A, C for example. So it should definitely be told, that the order of fragments matters and that the fragments are best used to specialise the configuration of other "layers" before. > However, what would be useful would be a way to dump the difference > between the current configuration, and what the combination of the > defconfig+fragments provide. > > I.e, you start Buildroot with omap2plus_defconfig + some specific > fragments as the kernel configuration. > > You run "make linux-show-config-diff", which returns empty. > > Then you tweak the configuration with "make linux-menuconfig", and run > "make linux-show-config-diff", which shows you the lines that you need > to put in one of your fragments to preserve the configuration tweaks > you have done. > > Of course, the name "linux-show-config-diff" is just made up here, and > some more thought is needed to find a good name. But overall, I > believe the semantic would be much clearer, and doesn't make any > assumption on whether we have one or several fragments, and whether > the last fragment or any other fragment needs to be updated. > > What do you think about this? Would this also fill your needs? Despite the fact, that you now have to put this diff manually somewhere it would not change things. Because this diff, as I pointed out above, is in general only useful for the last fragment. Updating any other fragment in the list would require either to have no dependencies between fragments at all (too restrictive at least for my use cases and also I'm not sure that this is feasible) or to have a deep understanding of the dependencies. From my perspective, neither option is that encouraging, so I do not want to propose a way to update any fragment other than the last one. Regards, Marcel Geschäftsführung: Susanne Kunschert, Thomas Pilz Pilz GmbH & Co. KG, Sitz: Ostfildern, HRA 210 893, Amtsgericht Stuttgart Kompl. Ges. Peter Pilz GmbH, Sitz: Ostfildern, HRB 210 612, Amtsgericht Stuttgart Umsatzsteuer: ID-Nr. DE 145 355 773, WEEE-Reg.-Nr. DE 71636849 This email is intended solely for the use of the named address(es). Any unauthorised disclosure, copying or distribution of these confidential information contained therein, or the taking of any action based on it, is prohibited. The sender disclaims any liability for the integrity of this email. Legally binding declarations must be in written form. Umweltschutz liegt uns am Herzen! - Bitte denken Sie an unsere Umwelt, bevor Sie diese E-Mail drucken. We do care about the environment! - Please consider the environment before printing this e-mail.
Hello Marcel, On Tue, 31 Jul 2018 09:44:19 +0200, Marcel Patzlaff wrote: > Hello Thomas, > > On my side, I find the semantic of "let's update the _last_ fragment" > > to be a bit weird and complex to understand. Why the last one, and not > > any other ? > > > > I'm not sure it's possible to have something that updates fragments > > with a sensible semantic. > > Well, it should just reflect what the merge_config script does: the last > file in the list of files to be merged always overrides settings done in > files before. So this means, that the last fragment is basically the > most significate one. Fragments are not about "priority", they are typically about splitting by topic different aspects. So it is not because the last fragment overrides the configuration options of other fragments that a configuration change you just did to enable driver FOO should go in the last fragment. > When I think about it, this should also be documented somewhere. Because > the result with configuration fragments A, B, C could be different than > B, A, C for example. So it should definitely be told, that the order of > fragments matters and that the fragments are best used to specialise the > configuration of other "layers" before. This can be documented in the Config.in help text of the BR2_LINUX_KERNEL_CONFIG_FRAGMENT_FILES option (ditto for U-Boot, etc.). > > However, what would be useful would be a way to dump the difference > > between the current configuration, and what the combination of the > > defconfig+fragments provide. > > > > I.e, you start Buildroot with omap2plus_defconfig + some specific > > fragments as the kernel configuration. > > > > You run "make linux-show-config-diff", which returns empty. > > > > Then you tweak the configuration with "make linux-menuconfig", and run > > "make linux-show-config-diff", which shows you the lines that you need > > to put in one of your fragments to preserve the configuration tweaks > > you have done. > > > > Of course, the name "linux-show-config-diff" is just made up here, and > > some more thought is needed to find a good name. But overall, I > > believe the semantic would be much clearer, and doesn't make any > > assumption on whether we have one or several fragments, and whether > > the last fragment or any other fragment needs to be updated. > > > > What do you think about this? Would this also fill your needs? > > Despite the fact, that you now have to put this diff manually somewhere > it would not change things. Because this diff, as I pointed out above, > is in general only useful for the last fragment. Not at all, as explained above. You enabled driver FOO, it may need to go to the last fragment, or potentially to any other fragment, it really depends on how the fragments are organized. > Updating any other fragment in the list would require either to have no > dependencies between fragments at all (too restrictive at least for my > use cases and also I'm not sure that this is feasible) or to have a deep > understanding of the dependencies. When someone is using fragments, then they understand what they mean to split the configuration in different snippets. People who don't understand that will typically use a single monolithic .config file. > From my perspective, neither option is that encouraging, so I do not > want to propose a way to update any fragment other than the last one. My position is that updating anything is not correct, because you cannot know whether it is the last fragment or any of the other fragment that needs to be updated. The only sane semantic is to show what the differences are, and let the user figure out where these configuration differences should be propagated. Best regards, Thomas
> > Well, it should just reflect what the merge_config script does: the last > > file in the list of files to be merged always overrides settings done in > > files before. So this means, that the last fragment is basically the > > most significate one. > > Fragments are not about "priority", they are typically about splitting > by topic different aspects. So it is not because the last fragment > overrides the configuration options of other fragments that a > configuration change you just did to enable driver FOO should go in the > last fragment. Fair enough, that's a use case where you most probably do not want to use the proposed automatism. But if you exploit the layered customisations (through BR2_EXTERNAL) and associate at most one fragment with each layer than you know which layer to update and you can make use of the proposed feature. That's at least how we are using it here. It would then just be analogous to <pkg>-update-defconfig in the first layer. > > Updating any other fragment in the list would require either to have no > > dependencies between fragments at all (too restrictive at least for my > > use cases and also I'm not sure that this is feasible) or to have a deep > > understanding of the dependencies. > > When someone is using fragments, then they understand what they mean to > split the configuration in different snippets. People who don't > understand that will typically use a single monolithic .config file. Of course one can keep it that way. But with the proposed change here it does not have to. > > From my perspective, neither option is that encouraging, so I do not > > want to propose a way to update any fragment other than the last one. > > My position is that updating anything is not correct, because you > cannot know whether it is the last fragment or any of the other > fragment that needs to be updated. The only sane semantic is to show > what the differences are, and let the user figure out where these > configuration differences should be propagated. The target clearily states what it updates. So for use cases where this is not meaningful it may of course not be used. If this change set is targeted at too specific use cases or does only solve one specific need, then well you should reject it. I am not sure about that because I only see how buildroot is used in-house. And from that perspective there is really a need for it. So at last, I do not have a problem withdrawing the patch series and keep maintaining it in-house. I only have to know if there is interest in it outside or not. Kind regards, Marcel Geschäftsführung: Susanne Kunschert, Thomas Pilz Pilz GmbH & Co. KG, Sitz: Ostfildern, HRA 210 893, Amtsgericht Stuttgart Kompl. Ges. Peter Pilz GmbH, Sitz: Ostfildern, HRB 210 612, Amtsgericht Stuttgart Umsatzsteuer: ID-Nr. DE 145 355 773, WEEE-Reg.-Nr. DE 71636849 This email is intended solely for the use of the named address(es). Any unauthorised disclosure, copying or distribution of these confidential information contained therein, or the taking of any action based on it, is prohibited. The sender disclaims any liability for the integrity of this email. Legally binding declarations must be in written form. Umweltschutz liegt uns am Herzen! - Bitte denken Sie an unsere Umwelt, bevor Sie diese E-Mail drucken. We do care about the environment! - Please consider the environment before printing this e-mail.
Thomas, Marcel, All, On 2018-07-30 23:46 +0200, Thomas Petazzoni spake thusly: > On Mon, 30 Jul 2018 17:51:53 +0200, Marcel Patzlaff wrote: > > This patch adds the new target above which implements the update routine as > > detailled in the head of this patch series. > > > > Signed-off-by: Marcel Patzlaff <m.patzlaff@pilz.de> > > Thanks for working on this complex topic, as Arnout said. > > On my side, I find the semantic of "let's update the _last_ fragment" > to be a bit weird and complex to understand. Why the last one, and not > any other ? > > I'm not sure it's possible to have something that updates fragments > with a sensible semantic. > > However, what would be useful would be a way to dump the difference > between the current configuration, and what the combination of the > defconfig+fragments provide. > > I.e, you start Buildroot with omap2plus_defconfig + some specific > fragments as the kernel configuration. > > You run "make linux-show-config-diff", which returns empty. > > Then you tweak the configuration with "make linux-menuconfig", and run > "make linux-show-config-diff", which shows you the lines that you need > to put in one of your fragments to preserve the configuration tweaks > you have done. > > Of course, the name "linux-show-config-diff" is just made up here, and > some more thought is needed to find a good name. But overall, I believe > the semantic would be much clearer, and doesn't make any assumption on > whether we have one or several fragments, and whether the last fragment > or any other fragment needs to be updated. > > What do you think about this? Would this also fill your needs? I would advocate that we do a generic solution like Thomas suggests, because it is generic enough that it would help solve more use-cases than this patch does. So, I side with Thomas here. I would just suggest that we do not add any new rule, but trying to update the defconfig when there are fragments would fail as it currently does, but also would display the delta if there is one. I.e.: $ make linux-menuconfig [change stuff] $ make linux-update-defconfig Unable to perform linux-update-defconfig when fragment files are set Configuration changes that you want to propagate to one of the fragments: -CONFIG_FOO=y +# CONFIG_BAR is unset linux/linux.mk:511: recipe for target 'linux-update-defconfig' failed make[1]: *** [linux-update-defconfig] Error 1 Regards, Yann E. MORIN.
Hello, Adding Arnout and Peter in Cc, in case they want to give their opinion on the patch series. On Tue, 31 Jul 2018 17:49:48 +0200, Yann E. MORIN wrote: > I would just suggest that we do not add any new rule, but trying to > update the defconfig when there are fragments would fail as it currently > does, but also would display the delta if there is one. I.e.: > > $ make linux-menuconfig > [change stuff] > $ make linux-update-defconfig > Unable to perform linux-update-defconfig when fragment files are set > Configuration changes that you want to propagate to one of the fragments: > -CONFIG_FOO=y > +# CONFIG_BAR is unset > linux/linux.mk:511: recipe for target 'linux-update-defconfig' failed > make[1]: *** [linux-update-defconfig] Error 1 I think we could do that in *addition* to having a new rule. Indeed, when you know what you're doing, having to run something that doesn't make sense ("make linux-update-defconfig") and which causes a failure is a bit silly. I'd rather run "make linux-diff-config" (or whatever name we chose). Marcel, do you think you could rework your patch series to go in the direction of showing a diff rather than arbitrarily adjusting the last fragment ? Thanks! Thomas
Thomas, Marcel, All, On 2018-08-14 16:27 +0200, Thomas Petazzoni spake thusly: > Adding Arnout and Peter in Cc, in case they want to give their opinion > on the patch series. > On Tue, 31 Jul 2018 17:49:48 +0200, Yann E. MORIN wrote: > > I would just suggest that we do not add any new rule, but trying to > > update the defconfig when there are fragments would fail as it currently > > does, but also would display the delta if there is one. I.e.: > > > > $ make linux-menuconfig > > [change stuff] > > $ make linux-update-defconfig > > Unable to perform linux-update-defconfig when fragment files are set > > Configuration changes that you want to propagate to one of the fragments: > > -CONFIG_FOO=y > > +# CONFIG_BAR is unset > > linux/linux.mk:511: recipe for target 'linux-update-defconfig' failed > > make[1]: *** [linux-update-defconfig] Error 1 > > I think we could do that in *addition* to having a new rule. Indeed, > when you know what you're doing, having to run something that doesn't > make sense ("make linux-update-defconfig") and which causes a failure > is a bit silly. I'd rather run "make linux-diff-config" (or whatever > name we chose). OK by me. > Marcel, do you think you could rework your patch series to go in the > direction of showing a diff rather than arbitrarily adjusting the last > fragment ? Yes, please! ;-) Regards, Yann E. MORIN. > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com
On 14-08-18 16:27, Thomas Petazzoni wrote: > Hello, > > Adding Arnout and Peter in Cc, in case they want to give their opinion > on the patch series. > > On Tue, 31 Jul 2018 17:49:48 +0200, Yann E. MORIN wrote: > >> I would just suggest that we do not add any new rule, but trying to >> update the defconfig when there are fragments would fail as it currently >> does, but also would display the delta if there is one. I.e.: >> >> $ make linux-menuconfig >> [change stuff] >> $ make linux-update-defconfig >> Unable to perform linux-update-defconfig when fragment files are set >> Configuration changes that you want to propagate to one of the fragments: >> -CONFIG_FOO=y >> +# CONFIG_BAR is unset >> linux/linux.mk:511: recipe for target 'linux-update-defconfig' failed >> make[1]: *** [linux-update-defconfig] Error 1 > > I think we could do that in *addition* to having a new rule. Indeed, > when you know what you're doing, having to run something that doesn't > make sense ("make linux-update-defconfig") and which causes a failure > is a bit silly. I'd rather run "make linux-diff-config" (or whatever > name we chose). > > Marcel, do you think you could rework your patch series to go in the > direction of showing a diff rather than arbitrarily adjusting the last > fragment ? Note that the diff really applies to the last fragment only, in the sense that if the last fragment is setting CONFIG_BAR, the "is unset" bit has to come after it. I don't think there is a way to say anything sensible automatically except for the last fragment. Regards, Arnout
Hello, On Wed, 15 Aug 2018 01:20:29 +0200, Arnout Vandecappelle wrote: > > Marcel, do you think you could rework your patch series to go in the > > direction of showing a diff rather than arbitrarily adjusting the last > > fragment ? > > Note that the diff really applies to the last fragment only, in the sense that > if the last fragment is setting CONFIG_BAR, the "is unset" bit has to come after > it. I don't think there is a way to say anything sensible automatically except > for the last fragment. Yes, the diff cannot be blindly applied to anything but the last fragment. But the idea in showing the diff is not for the user to apply it blindly, but rather to determine which parts of the diff should be reflected to each fragment. Best regards, Thomas
Thomas, Arnout, Marcel, All, On 2018-08-15 14:04 +0200, Thomas Petazzoni spake thusly: > On Wed, 15 Aug 2018 01:20:29 +0200, Arnout Vandecappelle wrote: > > > Marcel, do you think you could rework your patch series to go in the > > > direction of showing a diff rather than arbitrarily adjusting the last > > > fragment ? > > > > Note that the diff really applies to the last fragment only, in the sense that > > if the last fragment is setting CONFIG_BAR, the "is unset" bit has to come after > > it. I don't think there is a way to say anything sensible automatically except > > for the last fragment. > > Yes, the diff cannot be blindly applied to anything but the last > fragment. But the idea in showing the diff is not for the user to apply > it blindly, but rather to determine which parts of the diff should be > reflected to each fragment. Indeed, it is not to be considered to be a diff of any of the fragment, but of the configuration *as a whole*. That's why I initially did not write a full diff, but only the delta snippet, not the context, because it is a diff, not a patch. But I think we should instead use the scripts/diffconfig script that is in the linux kernel tree, and which outputs looks like: $ cat config.000 CONFIG_FOO=y $ cat config.001 CONFIG_FOO is not set $ ./scripts/diffconfig config.000 config.001 FOO y -> n which IMNSHO is exactly the message we want to convey. Regards, Yann E. MORIN. > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com
Hello, On Wed, 15 Aug 2018 18:16:57 +0200, Yann E. MORIN wrote: > Indeed, it is not to be considered to be a diff of any of the fragment, > but of the configuration *as a whole*. > > That's why I initially did not write a full diff, but only the delta > snippet, not the context, because it is a diff, not a patch. > > But I think we should instead use the scripts/diffconfig script that is > in the linux kernel tree, and which outputs looks like: > > $ cat config.000 > CONFIG_FOO=y > $ cat config.001 > CONFIG_FOO is not set > $ ./scripts/diffconfig config.000 config.001 > FOO y -> n > > which IMNSHO is exactly the message we want to convey. Indeed, this looks very good, and diffconfig is an existing tool. Thomas
Thomas, All, On 2018-08-15 19:35 +0200, Thomas Petazzoni spake thusly: > On Wed, 15 Aug 2018 18:16:57 +0200, Yann E. MORIN wrote: > > Indeed, it is not to be considered to be a diff of any of the fragment, > > but of the configuration *as a whole*. > > > > That's why I initially did not write a full diff, but only the delta > > snippet, not the context, because it is a diff, not a patch. > > > > But I think we should instead use the scripts/diffconfig script that is > > in the linux kernel tree, and which outputs looks like: > > > > $ cat config.000 > > CONFIG_FOO=y > > $ cat config.001 > > CONFIG_FOO is not set > > $ ./scripts/diffconfig config.000 config.001 > > FOO y -> n > > > > which IMNSHO is exactly the message we want to convey. > > Indeed, this looks very good, and diffconfig is an existing tool. Note that this was the diffconfig tool frm the kernel. The one we have behave differently (and badly I think), given the same config files as input: $ ./utils/diffconfig linux-config.000 linux-config.001 +IG_FOO n Which is not nice... :-/ Regards, Yann E. MORIN.
Hello, On Wed, 15 Aug 2018 22:29:22 +0200, Yann E. MORIN wrote: > > Indeed, this looks very good, and diffconfig is an existing tool. > > Note that this was the diffconfig tool frm the kernel. The one we have > behave differently (and badly I think), given the same config files as > input: > > $ ./utils/diffconfig linux-config.000 linux-config.001 > +IG_FOO n > > Which is not nice... :-/ Well, I guess we can import the diffconfig from the kernel, no? Thomas
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk index 331ba16614..bc817c457f 100644 --- a/package/pkg-kconfig.mk +++ b/package/pkg-kconfig.mk @@ -48,6 +48,18 @@ define kconfig-package-regen-dot-config $(Q)(yes "" | $($(1)_KCONFIG_MAKE) oldconfig))) endef +# Macro to create a .config file where all fragments are merged into +# $(1): the name of the package in upper-case letters +# $(2): name of the .config file +# $(3): fragment files to merge +define kconfig-package-merge-config + $(Q)$(if $($(1)_KCONFIG_DEFCONFIG),\ + $($(1)_KCONFIG_MAKE) $($(1)_KCONFIG_DEFCONFIG),\ + $(INSTALL) -m 0644 -D $($(1)_KCONFIG_FILE) $(2)) + $(Q)support/kconfig/merge_config.sh -m -O $(@D) $(2) $(3) + $(call kconfig-package-regen-dot-config,$(1)) +endef + ################################################################################ # inner-kconfig-package -- generates the make targets needed to support a # kconfig package @@ -116,12 +128,7 @@ $(2)_KCONFIG_RULES = \ # Since the file could be a defconfig file it needs to be expanded to a # full .config first. $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG): $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES) - $$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \ - $$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \ - $$(INSTALL) -m 0644 -D $$($(2)_KCONFIG_FILE) $$(@)) - $$(Q)support/kconfig/merge_config.sh -m -O $$(@D) \ - $$(@) $$($(2)_KCONFIG_FRAGMENT_FILES) - $$(call kconfig-package-regen-dot-config,$(2)) + $$(call kconfig-package-merge-config,$(2),$$(@),$$($(2)_KCONFIG_FRAGMENT_FILES)) # If _KCONFIG_FILE or _KCONFIG_FRAGMENT_FILES exists, this dependency is # already implied, but if we only have a _KCONFIG_DEFCONFIG we have to add @@ -239,11 +246,40 @@ $(1)-update-defconfig: PKG=$(2) $(1)-update-defconfig: $(1)-savedefconfig $$(call kconfig-package-update-config,defconfig) + +define $(2)_KCONFIG_LAST_FRAGMENT + $$(lastword $$($(2)_KCONFIG_FRAGMENT_FILES)) +endef + +define $(2)_KCONFIG_FIRST_FRAGMENTS + $$(filter-out $$($(2)_KCONFIG_LAST_FRAGMENT),\ + $$($(2)_KCONFIG_FRAGMENT_FILES)) +endef + +$(1)-update-last-config-fragment: PKG=$(2) +$(1)-update-last-config-fragment: $(1)-savedefconfig + @$$(if $$($(2)_KCONFIG_FRAGMENT_FILES), true, \ + echo "Unable to perform $(@) when no fragment files are set"; exit 1) + $$(Q)cp -a $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$($(2)_DIR)/.config.ulcf.all + $$(Q)cp -a $$($(2)_DIR)/defconfig $$($(2)_DIR)/defconfig.ulcf.all + $$(call kconfig-package-merge-config,$(2),$$($(2)_KCONFIG_DOTCONFIG),\ + $$($(2)_KCONFIG_FIRST_FRAGMENTS)) + $$(call kconfig-package-savedefconfig) + $$(Q)cp -a $$($(2)_DIR)/defconfig $$($(2)_DIR)/defconfig.ulcf.first + $$(Q)cp -a $$($(2)_DIR)/.config.ulcf.all $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) + $$(Q)cp -a $$($(2)_DIR)/defconfig.ulcf.all $$($(2)_DIR)/defconfig + $$(Q)utils/diff_defconfig.sh $$($(2)_DIR)/defconfig.ulcf.first \ + $$($(2)_DIR)/defconfig.ulcf.all $$($(2)_DIR)/.config.ulcf.all > \ + $$($(2)_KCONFIG_LAST_FRAGMENT) + $$(Q)rm -f $$($(2)_DIR)/defconfig.ulcf.* $$($(2)_DIR)/.config.ulcf.* + + endif # package enabled .PHONY: \ $(1)-update-config \ $(1)-update-defconfig \ + $(1)-update-last-config-fragment \ $(1)-savedefconfig \ $(1)-check-configuration-done \ $$($(2)_DIR)/.kconfig_editor_% \
This patch adds the new target above which implements the update routine as detailled in the head of this patch series. Signed-off-by: Marcel Patzlaff <m.patzlaff@pilz.de> --- package/pkg-kconfig.mk | 48 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-)