What is a useful way to show changes between series?

What is a useful way to show changes between series?

[Konstantin Ryabitsev]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

Hi, all:

What would be a useful way to quickly do a "show me what changed" 
between two versions of the same patch series? Especially considering 
that:

- there may be a different number of patches in the series
- new revisions will likely be based on newer trees
- subjects/commit messages/trailers may be vastly different between two 
  series, even if the code is very similar (e.g. someone broke down a 
  single large commit into several smaller ones or vice-versa)

In my mind, the best way to approach this would be:

1. Create a single unified diff file from all patches in each series 
   (using combinediff)
2. Remove all context, leaving only -/+ lines (using interdiff -U0)
3. Remove all line numbers from @@ @@ hunks (replacing them with 0)
4. Add commit information on top of the diff

In the end, we get two summary documents that can be compared using 
regular diffing tools like vimdiff/meld/etc and actually show a fairly 
succinct "how are these two series different" view.

The goal is not to replace code review, obviously, but to give a way for 
a maintainer to do a quick sanity-check of a series they've reviewed 
before.

I ran this scenario on a random patch series that I found 
(https://lore.kernel.org/lkml/b2b9cb06-b0eb-3e94-b83a-e58467069ae2@gmail.com/) 
and attached are two summary files:

 - phy_v2.summary
 - phy_v3.summary

Here's how diffing them looks in vimdiff and meld:

 - https://mricon.com/misc/phy-summary-vimdiff-fs8.png
 - https://mricon.com/misc/phy-summary-meld-fs8.png

So, questions for you are:

1. Is this is a useful way to present an "at-a-glance" view?
2. If yes, how would you change it to be more useful?
3. If not a good approach at all, how would you recommend approaching it 
   instead?

Thanks,
-K

[-- Attachment #2: phy_v2.summary --]
[-- Type: text/plain, Size: 7935 bytes --]

[PATCH 1/3]
    From: Robert Marko <robert.marko@sartura.hr>
    Subject: net: phy: mdio: add IPQ40xx MDIO driver

    This patch adds the driver for the MDIO interface
    inside of Qualcomm IPQ40xx series SoC-s.

    Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
    Signed-off-by: Robert Marko <robert.marko@sartura.hr>
    Cc: Luka Perkov <luka.perkov@sartura.hr>

[PATCH 2/3]
    From: Robert Marko <robert.marko@sartura.hr>
    Subject: dt-bindings: add Qualcomm IPQ4019 MDIO bindings

    This patch adds the binding document for the IPQ40xx MDIO driver.

    Signed-off-by: Robert Marko <robert.marko@sartura.hr>
    Cc: Luka Perkov <luka.perkov@sartura.hr>

[PATCH 3/3]
    From: Robert Marko <robert.marko@sartura.hr>
    Subject: dts: ipq4019: add MDIO node

    This patch adds the necessary MDIO interface node
    to the Qualcomm IPQ4019 DTSI.

    Built-in QCA8337N switch is managed using it,
    and since we have a driver for it lets add it.

    Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
    Signed-off-by: Robert Marko <robert.marko@sartura.hr>
    Cc: Luka Perkov <luka.perkov@sartura.hr>

--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -0,0 +0,7 @@
+config MDIO_IPQ40XX
+	tristate "Qualcomm IPQ40xx MDIO interface"
+	depends on HAS_IOMEM && OF
+	help
+	  This driver supports the MDIO interface found in Qualcomm
+	  IPQ40xx series Soc-s.
+
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -0,0 +0 @@
+obj-$(CONFIG_MDIO_IPQ40XX)	+= mdio-ipq40xx.o
--- /dev/null
+++ b/drivers/net/phy/mdio-ipq40xx.c
@@ -0,0 +0,176 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2020 Sartura Ltd. */
+
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+
+#define MDIO_CTRL_0_REG		0x40
+#define MDIO_CTRL_1_REG		0x44
+#define MDIO_CTRL_2_REG		0x48
+#define MDIO_CTRL_3_REG		0x4c
+#define MDIO_CTRL_4_REG		0x50
+#define MDIO_CTRL_4_ACCESS_BUSY		BIT(16)
+#define MDIO_CTRL_4_ACCESS_START		BIT(8)
+#define MDIO_CTRL_4_ACCESS_CODE_READ		0
+#define MDIO_CTRL_4_ACCESS_CODE_WRITE	1
+
+#define IPQ40XX_MDIO_RETRY	1000
+#define IPQ40XX_MDIO_DELAY	10
+
+struct ipq40xx_mdio_data {
+	void __iomem	*membase;
+};
+
+static int ipq40xx_mdio_wait_busy(struct mii_bus *bus)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	int i;
+
+	for (i = 0; i < IPQ40XX_MDIO_RETRY; i++) {
+		unsigned int busy;
+
+		busy = readl(priv->membase + MDIO_CTRL_4_REG) &
+			MDIO_CTRL_4_ACCESS_BUSY;
+		if (!busy)
+			return 0;
+
+		/* BUSY might take to be cleard by 15~20 times of loop */
+		udelay(IPQ40XX_MDIO_DELAY);
+	}
+
+	dev_err(bus->parent, "MDIO operation timed out\n");
+
+	return -ETIMEDOUT;
+}
+
+static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	int value = 0;
+	unsigned int cmd = 0;
+
+	/* Reject clause 45 */
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* issue the phy address and reg */
+	writel((mii_id << 8) | regnum, priv->membase + MDIO_CTRL_1_REG);
+
+	cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_READ;
+
+	/* issue read command */
+	writel(cmd, priv->membase + MDIO_CTRL_4_REG);
+
+	/* Wait read complete */
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* Read data */
+	value = readl(priv->membase + MDIO_CTRL_3_REG);
+
+	return value;
+}
+
+static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
+							 u16 value)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	unsigned int cmd = 0;
+
+	/* Reject clause 45 */
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* issue the phy address and reg */
+	writel((mii_id << 8) | regnum, priv->membase + MDIO_CTRL_1_REG);
+
+	/* issue write data */
+	writel(value, priv->membase + MDIO_CTRL_2_REG);
+
+	cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_WRITE;
+	/* issue write command */
+	writel(cmd, priv->membase + MDIO_CTRL_4_REG);
+
+	/* Wait write complete */
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int ipq40xx_mdio_probe(struct platform_device *pdev)
+{
+	struct ipq40xx_mdio_data *priv;
+	struct mii_bus *bus;
+	int ret;
+
+	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
+	if (!bus)
+		return -ENOMEM;
+
+	priv = bus->priv;
+
+	priv->membase = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->membase))
+		return PTR_ERR(priv->membase);
+
+	bus->name = "ipq40xx_mdio";
+	bus->read = ipq40xx_mdio_read;
+	bus->write = ipq40xx_mdio_write;
+	bus->parent = &pdev->dev;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
+
+	ret = of_mdiobus_register(bus, pdev->dev.of_node);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, bus);
+
+	return 0;
+}
+
+static int ipq40xx_mdio_remove(struct platform_device *pdev)
+{
+	struct mii_bus *bus = platform_get_drvdata(pdev);
+
+	mdiobus_unregister(bus);
+
+	return 0;
+}
+
+static const struct of_device_id ipq40xx_mdio_dt_ids[] = {
+	{ .compatible = "qcom,ipq40xx-mdio" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ipq40xx_mdio_dt_ids);
+
+static struct platform_driver ipq40xx_mdio_driver = {
+	.probe = ipq40xx_mdio_probe,
+	.remove = ipq40xx_mdio_remove,
+	.driver = {
+		.name = "ipq40xx-mdio",
+		.of_match_table = ipq40xx_mdio_dt_ids,
+	},
+};
+
+module_platform_driver(ipq40xx_mdio_driver);
+
+MODULE_DESCRIPTION("IPQ40XX MDIO interface driver");
+MODULE_AUTHOR("Qualcomm Atheros");
+MODULE_LICENSE("Dual BSD/GPL");
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
@@ -0,0 +0,62 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qcom,ipq40xx-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+allOf:
+  - $ref: "mdio.yaml#"
+
+properties:
+  compatible:
+    const: qcom,ipq40xx-mdio
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+examples:
+  - |
+    mdio@90000 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "qcom,ipq40xx-mdio";
+      reg = <0x90000 0x64>;
+      status = "disabled";
+
+      ethphy0: ethernet-phy@0 {
+        reg = <0>;
+      };
+
+      ethphy1: ethernet-phy@1 {
+        reg = <1>;
+      };
+
+      ethphy2: ethernet-phy@2 {
+        reg = <2>;
+      };
+
+      ethphy3: ethernet-phy@3 {
+        reg = <3>;
+      };
+
+      ethphy4: ethernet-phy@4 {
+        reg = <4>;
+      };
+    };
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -0,0 +0,28 @@
+		mdio: mdio@90000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "qcom,ipq40xx-mdio";
+			reg = <0x90000 0x64>;
+			status = "disabled";
+
+			ethphy0: ethernet-phy@0 {
+				reg = <0>;
+			};
+
+			ethphy1: ethernet-phy@1 {
+				reg = <1>;
+			};
+
+			ethphy2: ethernet-phy@2 {
+				reg = <2>;
+			};
+
+			ethphy3: ethernet-phy@3 {
+				reg = <3>;
+			};
+
+			ethphy4: ethernet-phy@4 {
+				reg = <4>;
+			};
+		};
+

[-- Attachment #3: phy_v3.summary --]
[-- Type: text/plain, Size: 7643 bytes --]

[PATCH 1/3]
    From: Robert Marko <robert.marko@sartura.hr>
    Subject: net: phy: mdio: add IPQ40xx MDIO driver

    This patch adds the driver for the MDIO interface
    inside of Qualcomm IPQ40xx series SoC-s.

    Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
    Signed-off-by: Robert Marko <robert.marko@sartura.hr>
    Cc: Luka Perkov <luka.perkov@sartura.hr>

[PATCH 2/3]
    From: Robert Marko <robert.marko@sartura.hr>
    Subject: dt-bindings: add Qualcomm IPQ4019 MDIO bindings

    This patch adds the binding document for the IPQ40xx MDIO driver.

    Signed-off-by: Robert Marko <robert.marko@sartura.hr>
    Cc: Luka Perkov <luka.perkov@sartura.hr>

[PATCH 3/3]
    From: Robert Marko <robert.marko@sartura.hr>
    Subject: ARM: dts: qcom: ipq4019: add MDIO node

    This patch adds the necessary MDIO interface node
    to the Qualcomm IPQ4019 DTSI.

    Built-in QCA8337N switch is managed using it,
    and since we have a driver for it lets add it.

    Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
    Signed-off-by: Robert Marko <robert.marko@sartura.hr>
    Cc: Luka Perkov <luka.perkov@sartura.hr>

--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -0,0 +0,7 @@
+config MDIO_IPQ40XX
+	tristate "Qualcomm IPQ40xx MDIO interface"
+	depends on HAS_IOMEM && OF_MDIO
+	help
+	  This driver supports the MDIO interface found in Qualcomm
+	  IPQ40xx series Soc-s.
+
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -0,0 +0 @@
+obj-$(CONFIG_MDIO_IPQ40XX)	+= mdio-ipq40xx.o
--- /dev/null
+++ b/drivers/net/phy/mdio-ipq40xx.c
@@ -0,0 +0,160 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2020 Sartura Ltd. */
+
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+
+#define MDIO_ADDR_REG				0x44
+#define MDIO_DATA_WRITE_REG			0x48
+#define MDIO_DATA_READ_REG			0x4c
+#define MDIO_CMD_REG				0x50
+#define MDIO_CMD_ACCESS_BUSY		BIT(16)
+#define MDIO_CMD_ACCESS_START		BIT(8)
+#define MDIO_CMD_ACCESS_CODE_READ	0
+#define MDIO_CMD_ACCESS_CODE_WRITE	1
+
+#define IPQ40XX_MDIO_TIMEOUT	10000
+#define IPQ40XX_MDIO_SLEEP		10
+
+struct ipq40xx_mdio_data {
+	void __iomem	*membase;
+};
+
+static int ipq40xx_mdio_wait_busy(struct mii_bus *bus)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	unsigned int busy;
+
+	return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
+				  (busy & MDIO_CMD_ACCESS_BUSY) == 0, 
+				  IPQ40XX_MDIO_SLEEP, IPQ40XX_MDIO_TIMEOUT);
+}
+
+static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	unsigned int cmd;
+
+	/* Reject clause 45 */
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* issue the phy address and reg */
+	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+
+	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_READ;
+
+	/* issue read command */
+	writel(cmd, priv->membase + MDIO_CMD_REG);
+
+	/* Wait read complete */
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* Read and return data */
+	return readl(priv->membase + MDIO_DATA_READ_REG);
+}
+
+static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
+							 u16 value)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	unsigned int cmd;
+
+	/* Reject clause 45 */
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* issue the phy address and reg */
+	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+
+	/* issue write data */
+	writel(value, priv->membase + MDIO_DATA_WRITE_REG);
+
+	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_WRITE;
+	/* issue write command */
+	writel(cmd, priv->membase + MDIO_CMD_REG);
+
+	/* Wait write complete */
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int ipq40xx_mdio_probe(struct platform_device *pdev)
+{
+	struct ipq40xx_mdio_data *priv;
+	struct mii_bus *bus;
+	int ret;
+
+	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
+	if (!bus)
+		return -ENOMEM;
+
+	priv = bus->priv;
+
+	priv->membase = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->membase))
+		return PTR_ERR(priv->membase);
+
+	bus->name = "ipq40xx_mdio";
+	bus->read = ipq40xx_mdio_read;
+	bus->write = ipq40xx_mdio_write;
+	bus->parent = &pdev->dev;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
+
+	ret = of_mdiobus_register(bus, pdev->dev.of_node);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, bus);
+
+	return 0;
+}
+
+static int ipq40xx_mdio_remove(struct platform_device *pdev)
+{
+	struct mii_bus *bus = platform_get_drvdata(pdev);
+
+	mdiobus_unregister(bus);
+
+	return 0;
+}
+
+static const struct of_device_id ipq40xx_mdio_dt_ids[] = {
+	{ .compatible = "qcom,ipq40xx-mdio" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ipq40xx_mdio_dt_ids);
+
+static struct platform_driver ipq40xx_mdio_driver = {
+	.probe = ipq40xx_mdio_probe,
+	.remove = ipq40xx_mdio_remove,
+	.driver = {
+		.name = "ipq40xx-mdio",
+		.of_match_table = ipq40xx_mdio_dt_ids,
+	},
+};
+
+module_platform_driver(ipq40xx_mdio_driver);
+
+MODULE_DESCRIPTION("IPQ40XX MDIO interface driver");
+MODULE_AUTHOR("Qualcomm Atheros");
+MODULE_LICENSE("Dual BSD/GPL");
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
@@ -0,0 +0,61 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qcom,ipq40xx-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings
+
+maintainers:
+  - Robert Marko <robert.marko@sartura.hr>
+
+allOf:
+  - $ref: "mdio.yaml#"
+
+properties:
+  compatible:
+    const: qcom,ipq40xx-mdio
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+examples:
+  - |
+    mdio@90000 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "qcom,ipq40xx-mdio";
+      reg = <0x90000 0x64>;
+
+      ethphy0: ethernet-phy@0 {
+        reg = <0>;
+      };
+
+      ethphy1: ethernet-phy@1 {
+        reg = <1>;
+      };
+
+      ethphy2: ethernet-phy@2 {
+        reg = <2>;
+      };
+
+      ethphy3: ethernet-phy@3 {
+        reg = <3>;
+      };
+
+      ethphy4: ethernet-phy@4 {
+        reg = <4>;
+      };
+    };
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -0,0 +0,28 @@
+		mdio: mdio@90000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "qcom,ipq40xx-mdio";
+			reg = <0x90000 0x64>;
+			status = "disabled";
+
+			ethphy0: ethernet-phy@0 {
+				reg = <0>;
+			};
+
+			ethphy1: ethernet-phy@1 {
+				reg = <1>;
+			};
+
+			ethphy2: ethernet-phy@2 {
+				reg = <2>;
+			};
+
+			ethphy3: ethernet-phy@3 {
+				reg = <3>;
+			};
+
+			ethphy4: ethernet-phy@4 {
+				reg = <4>;
+			};
+		};
+

Re: [kernel.org users] What is a useful way to show changes between series?

[Jason Gunthorpe]

On Tue, Apr 28, 2020 at 03:29:38PM -0400, Konstantin Ryabitsev wrote:
> Hi, all:
> 
> What would be a useful way to quickly do a "show me what changed" 
> between two versions of the same patch series? Especially considering 
> that:
> 
> - there may be a different number of patches in the series
> - new revisions will likely be based on newer trees
> - subjects/commit messages/trailers may be vastly different between two 
>   series, even if the code is very similar (e.g. someone broke down a 
>   single large commit into several smaller ones or vice-versa)
>
> In my mind, the best way to approach this would be:
> 
> 1. Create a single unified diff file from all patches in each series 
>    (using combinediff)
> 2. Remove all context, leaving only -/+ lines (using interdiff -U0)
> 3. Remove all line numbers from @@ @@ hunks (replacing them with 0)
> 4. Add commit information on top of the diff

git range-diff is not so bad at visualizing this

Jason

Re: [kernel.org users] What is a useful way to show changes between series?

[Konstantin Ryabitsev]

On Tue, Apr 28, 2020 at 04:35:50PM -0300, Jason Gunthorpe wrote:
> > - there may be a different number of patches in the series
> > - new revisions will likely be based on newer trees
> > - subjects/commit messages/trailers may be vastly different between two 
> >   series, even if the code is very similar (e.g. someone broke down a 
> >   single large commit into several smaller ones or vice-versa)
> >
> > In my mind, the best way to approach this would be:
> > 
> > 1. Create a single unified diff file from all patches in each series 
> >    (using combinediff)
> > 2. Remove all context, leaving only -/+ lines (using interdiff -U0)
> > 3. Remove all line numbers from @@ @@ hunks (replacing them with 0)
> > 4. Add commit information on top of the diff
> 
> git range-diff is not so bad at visualizing this

Right, but that only works from a tree, correct? Unfortunately, very few 
people are providing base-commit information still, so I'm not sure how 
reliably we could generate a range-diff between series if we don't know 
where in the tree they belong, which is why I was looking at easy ways 
of accomplishing something like this before touching git at all.

If everyone is already using range-diff for this purpose and I'm 
attempting to solve a problem that already has a widely used solution, 
then please also let me know. :)

Thanks,
-K

Re: [kernel.org users] What is a useful way to show changes between series?

[Jason Gunthorpe]

On Tue, Apr 28, 2020 at 03:44:19PM -0400, Konstantin Ryabitsev wrote:
> On Tue, Apr 28, 2020 at 04:35:50PM -0300, Jason Gunthorpe wrote:
> > > - there may be a different number of patches in the series
> > > - new revisions will likely be based on newer trees
> > > - subjects/commit messages/trailers may be vastly different between two 
> > >   series, even if the code is very similar (e.g. someone broke down a 
> > >   single large commit into several smaller ones or vice-versa)
> > >
> > > In my mind, the best way to approach this would be:
> > > 
> > > 1. Create a single unified diff file from all patches in each series 
> > >    (using combinediff)
> > > 2. Remove all context, leaving only -/+ lines (using interdiff -U0)
> > > 3. Remove all line numbers from @@ @@ hunks (replacing them with 0)
> > > 4. Add commit information on top of the diff
> > 
> > git range-diff is not so bad at visualizing this
> 
> Right, but that only works from a tree, correct? Unfortunately, very few 
> people are providing base-commit information still, so I'm not sure how 
> reliably we could generate a range-diff between series if we don't know 
> where in the tree they belong, which is why I was looking at easy ways 
> of accomplishing something like this before touching git at all.

In this case the absolute tree isn't important, if you have the blobs
referenced in the diffs you can build commits from a dummy base and
run git-range-diff over that.

Alternatively you could try to duplicate the visualization it uses
without git commits?

Actually, come to think of it, I wrote some part of this once to solve
some other problem, look here:

https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_expand_am.py

> If everyone is already using range-diff for this purpose and I'm 
> attempting to solve a problem that already has a widely used solution, 
> then please also let me know. :)

It is what I usually reach for in the few cases I want to do this..

Jason

Re: [kernel.org users] What is a useful way to show changes between series?

[Konstantin Ryabitsev]

On Tue, Apr 28, 2020 at 04:55:55PM -0300, Jason Gunthorpe wrote:
> > Right, but that only works from a tree, correct? Unfortunately, very 
> > few people are providing base-commit information still, so I'm not 
> > sure how reliably we could generate a range-diff between series if 
> > we don't know where in the tree they belong, which is why I was 
> > looking at easy ways of accomplishing something like this before 
> > touching git at all.
> 
> In this case the absolute tree isn't important, if you have the blobs
> referenced in the diffs you can build commits from a dummy base and
> run git-range-diff over that.
> 
> Alternatively you could try to duplicate the visualization it uses
> without git commits?
> 
> Actually, come to think of it, I wrote some part of this once to solve
> some other problem, look here:
> 
> https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_expand_am.py

Interesting. Let me poke at this and see if I can do auto-range-diff 
between patch series even when we don't have base-commit info.

-K

Re: [tools] jg expand-am (Was: What is a useful way to show changes between series?)

[Konstantin Ryabitsev]

On Tue, Apr 28, 2020 at 04:55:55PM -0300, Jason Gunthorpe wrote:
> > Right, but that only works from a tree, correct? Unfortunately, very 
> > few people are providing base-commit information still, so I'm not 
> > sure how reliably we could generate a range-diff between series if 
> > we don't know where in the tree they belong, which is why I was 
> > looking at easy ways of accomplishing something like this before 
> > touching git at all.
> 
> In this case the absolute tree isn't important, if you have the blobs
> referenced in the diffs you can build commits from a dummy base and
> run git-range-diff over that.
> 
> Alternatively you could try to duplicate the visualization it uses
> without git commits?
> 
> Actually, come to think of it, I wrote some part of this once to solve
> some other problem, look here:
> 
> https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_expand_am.py

(Dropping users from this follow-up)

I'm trying to wrap my head around what `jg expand-am` does, and I need a 
bit of your help to figure out what happens after it completes. So, when 
you expand-am, you:

- grab a list of files and their index info
- create a temporary detached-head worktree without any files in it
- populate it with the exact files as in the patch, using the index info 
  to grab the blobs at the exact state they were when the patches were 
  created
- git am the patch/series
- display the tip commit of the "git am" operation
- destroy the temporary worktree

So, you now have these commits as loose objects in the original tree.
What happens next in your workflow? How do these objects help you during 
the actual "git am"?

(My git experience is pretty much entirely branch/merge based, so I'm in 
the dark about a lot of this -- thanks for helping me understand your 
perspective.)

-K

Re: [tools] jg expand-am (Was: What is a useful way to show changes between series?)

[Jason Gunthorpe]

On Wed, Apr 29, 2020 at 12:41:07PM -0400, Konstantin Ryabitsev wrote:
> So, you now have these commits as loose objects in the original tree.
> What happens next in your workflow? How do these objects help you during 
> the actual "git am"?

The commits are not the goal. The purpose is to populate the object
store with evey file blob mentioned in every diff header:

index a1f053cd30b990..49775cda83dc18 100644

ie the goal is to create blob 49775cda83dc18

This effects 'git am -3'. If it has the source blob then it does
standard 3 way git merge - otherwise it tries less hard and drops .rej
files.

The problem comes when a series of patches change the same file
multiple times. The file might go through blobs A->B->C->D over 4
patches.

I probably have the starting blob A, however, my top of tree has that
file as W. So my git am will produce blobs W->X->Y->Z

The key thing is that when am applies A->B on top of W it gets W->X. 
am never creates B.

Thus when am goes to do B->C it can't find B, and now it can't do 3
way. If there are conflicts you get .rej files.

This comes up from time to time and rather than asking to resend I can
use this script.

About half the time git 3 way merge will resolve things automatically,
the rest I prefer to use the conflict markers to fix up rather than
.rej files.

Jason

Re: [tools] jg expand-am (Was: What is a useful way to show changes between series?)

[Konstantin Ryabitsev]

On Wed, Apr 29, 2020 at 02:15:40PM -0300, Jason Gunthorpe wrote:
> The commits are not the goal. The purpose is to populate the object
> store with evey file blob mentioned in every diff header:
> 
> index a1f053cd30b990..49775cda83dc18 100644
> 
> ie the goal is to create blob 49775cda83dc18
> 
> This effects 'git am -3'. If it has the source blob then it does
> standard 3 way git merge - otherwise it tries less hard and drops .rej
> files.
> 
> The problem comes when a series of patches change the same file
> multiple times. The file might go through blobs A->B->C->D over 4
> patches.
> 
> I probably have the starting blob A, however, my top of tree has that
> file as W. So my git am will produce blobs W->X->Y->Z
> 
> The key thing is that when am applies A->B on top of W it gets W->X. 
> am never creates B.
> 
> Thus when am goes to do B->C it can't find B, and now it can't do 3
> way. If there are conflicts you get .rej files.

Ah, this is a pretty neat hack. It would be fairly easy for b4 to do the 
same thing if we're running "b4 am" inside a git tree -- I'll see if I 
can add that in as an optional "b4 am -3" flag.

Getting back to the original topic, I now better understand what you're 
suggesting. We perform the exact same operation on two different series 
(v1, v2) and then perform:

git range-diff <dummy-base-v1>..<dummy-tip-v1> <dummy-base-v2>..<dummy-tip-v2>

Assuming we were able to find all the blobs, this should effectively 
allow us to range-diff across arbitrary patchsets without knowing the 
base-commit. Neat.

Thanks for suggesting that -- I'll see if I can work that into b4.

-K

Re: [tools] jg expand-am (Was: What is a useful way to show changes between series?)

[Jason Gunthorpe]

On Wed, Apr 29, 2020 at 01:58:20PM -0400, Konstantin Ryabitsev wrote:

> Ah, this is a pretty neat hack. It would be fairly easy for b4 to do the 
> same thing if we're running "b4 am" inside a git tree -- I'll see if I 
> can add that in as an optional "b4 am -3" flag.

IMHO it should be part of git am, a --rebuild-blobs flag or
something. git am -3 doesn't really work as intended without this..
 
> Getting back to the original topic, I now better understand what you're 
> suggesting. We perform the exact same operation on two different series 
> (v1, v2) and then perform:
> 
> git range-diff <dummy-base-v1>..<dummy-tip-v1> <dummy-base-v2>..<dummy-tip-v2>
> 
> Assuming we were able to find all the blobs, this should effectively 
> allow us to range-diff across arbitrary patchsets without knowing the 
> base-commit. Neat.

Yep

For the purpose of range diff the files outside the series can be
ignored.

Jason

Re: [kernel.org users] What is a useful way to show changes between series?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

On Tue, Apr 28, 2020 at 03:44:19PM -0400, Konstantin Ryabitsev wrote:

> Right, but that only works from a tree, correct? Unfortunately, very few 
> people are providing base-commit information still, so I'm not sure how 

It would be helpful if git could be configured to do this by default
in format-patch rather than requiring it to be done manually on the
command line separately to specifying the set of patches to format.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]

Re: [kernel.org users] What is a useful way to show changes between series?

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

On Wed, 2020-04-29 at 11:13 +0100, Mark Brown wrote:
> On Tue, Apr 28, 2020 at 03:44:19PM -0400, Konstantin Ryabitsev wrote:
> 
> > Right, but that only works from a tree, correct? Unfortunately,
> > very few people are providing base-commit information still, so I'm
> > not sure how 
> 
> It would be helpful if git could be configured to do this by default
> in format-patch rather than requiring it to be done manually on the
> command line separately to specifying the set of patches to format.

It might also end up causing a lot of confusion:  remember, a lot of
people use a quilt like workflow for their patches ... I know I do
because when I've got several patch series to upport, having a single
branch with all of them is much easier to work with than multiple
branches and a combination.  Even in the simplest cases where I'm
working on the current tree and have only a single series, I often have
a bugfix for some current issue as the base patch so I can boot my
series easily.  In either case forcing the cover letter to show my base
patch would give you useless information because it wouldn't be in any
tree you have.

I really think a base commit is sometimes useful but definitely not
essential information.  Quilt is the poster child for tools that don't
use a base commit.  I like to think that base commit based tools like
git perform better under conflict merges, but I have to confess given
some of the merge messes I've seen, there's no substantive difference
between what git does and what quilt spits into .rej files.

James

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

Re: [kernel.org users] What is a useful way to show changes between series?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

On Wed, Apr 29, 2020 at 08:00:26AM -0700, James Bottomley wrote:
> On Wed, 2020-04-29 at 11:13 +0100, Mark Brown wrote:
> > On Tue, Apr 28, 2020 at 03:44:19PM -0400, Konstantin Ryabitsev wrote:

> > > Right, but that only works from a tree, correct? Unfortunately,
> > > very few people are providing base-commit information still, so I'm
> > > not sure how 

> > It would be helpful if git could be configured to do this by default
> > in format-patch rather than requiring it to be done manually on the
> > command line separately to specifying the set of patches to format.

> It might also end up causing a lot of confusion:  remember, a lot of
> people use a quilt like workflow for their patches ... I know I do
> because when I've got several patch series to upport, having a single
> branch with all of them is much easier to work with than multiple
> branches and a combination.  Even in the simplest cases where I'm
> working on the current tree and have only a single series, I often have
> a bugfix for some current issue as the base patch so I can boot my

I did say "default" rather than "force" - it's a lot easier if you have
a workflow where it might be useful if you can just turn it on instead
of having to manually go and specify the base commit twice when
formatting serieses which is both a pain and easy to forget, then
disable it where it doesn't make sense.  Right now it's more effort than
it's worth to use even when it does make sense.

> series easily.  In either case forcing the cover letter to show my base
> patch would give you useless information because it wouldn't be in any
> tree you have.

TBH it's fairly harmless if the commit that's being referenced doesn't
exist.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]

Re: [kernel.org users] What is a useful way to show changes between series?

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]

On Wed, 2020-04-29 at 16:19 +0100, Mark Brown wrote:
> On Wed, Apr 29, 2020 at 08:00:26AM -0700, James Bottomley wrote:
> > On Wed, 2020-04-29 at 11:13 +0100, Mark Brown wrote:
> > > On Tue, Apr 28, 2020 at 03:44:19PM -0400, Konstantin Ryabitsev
> > > wrote:
> > > > Right, but that only works from a tree, correct? Unfortunately,
> > > > very few people are providing base-commit information still, so
> > > > I'm not sure how 
> > > It would be helpful if git could be configured to do this by
> > > default in format-patch rather than requiring it to be done
> > > manually on the command line separately to specifying the set of
> > > patches to format.
> > It might also end up causing a lot of confusion:  remember, a lot
> > of people use a quilt like workflow for their patches ... I know I
> > do because when I've got several patch series to upport, having a
> > single branch with all of them is much easier to work with than
> > multiple branches and a combination.  Even in the simplest cases
> > where I'm working on the current tree and have only a single
> > series, I often have a bugfix for some current issue as the base
> > patch so I can boot my
> 
> I did say "default" rather than "force" - it's a lot easier if you
> have a workflow where it might be useful if you can just turn it on
> instead of having to manually go and specify the base commit twice
> when formatting serieses which is both a pain and easy to forget,
> then disable it where it doesn't make sense.  Right now it's more
> effort than it's worth to use even when it does make sense.
> 
> > series easily.  In either case forcing the cover letter to show my
> > base patch would give you useless information because it wouldn't
> > be in any tree you have.
> 
> TBH it's fairly harmless if the commit that's being referenced
> doesn't exist.

I think we need to be careful here.  Today if you specify a base
everyone assumes it should exist and they know something is seriously
wrong if it doesn't (0day will complain, for instance).  If you force
everyone to specify a base, then suddenly you lose the information that
they really meant it and you have no idea if a bogus base is an
indicator of problems with the series.  Forcing a default base would
make the base commit useless as an indicator of correctness since now
you have to process bogus bases as well.

Of course, this is simply an argument for still having the option, just
not making it default, so if you know what you're doing and don't use a
quilt like workflow, you turn it on.

James

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

Re: [kernel.org users] What is a useful way to show changes between series?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]

On Wed, Apr 29, 2020 at 08:44:30AM -0700, James Bottomley wrote:
> On Wed, 2020-04-29 at 16:19 +0100, Mark Brown wrote:

> > I did say "default" rather than "force" - it's a lot easier if you
> > have a workflow where it might be useful if you can just turn it on

> I think we need to be careful here.  Today if you specify a base
> everyone assumes it should exist and they know something is seriously
> wrong if it doesn't (0day will complain, for instance).  If you force
> everyone to specify a base, then suddenly you lose the information that
> they really meant it and you have no idea if a bogus base is an
> indicator of problems with the series.  Forcing a default base would
> make the base commit useless as an indicator of correctness since now
> you have to process bogus bases as well.

> Of course, this is simply an argument for still having the option, just
> not making it default, so if you know what you're doing and don't use a
> quilt like workflow, you turn it on.

This is why I suggested adding the ability to configure git to do this
by default rather than suggesting just doing this by default.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]

Re: [kernel.org users] What is a useful way to show changes between series?

[Junio C Hamano]

Are we talking about format.useAutoBase or something more elaborate?

2020年4月29日(水) 3:13 Mark Brown <broonie@kernel.org>:
>
> On Tue, Apr 28, 2020 at 03:44:19PM -0400, Konstantin Ryabitsev wrote:
>
> > Right, but that only works from a tree, correct? Unfortunately, very few
> > people are providing base-commit information still, so I'm not sure how
>
> It would be helpful if git could be configured to do this by default
> in format-patch rather than requiring it to be done manually on the
> command line separately to specifying the set of patches to format.
> 

Re: [kernel.org users] What is a useful way to show changes between series?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

On Wed, Apr 29, 2020 at 10:42:01AM -0700, Junio C Hamano wrote:

> Are we talking about format.useAutoBase or something more elaborate?

Ah, indeed I think that is exactly what I'm looking for!  Sadly it is
not documented in the git format-patch man page so I hadn't seen it,
even the base auto option is a bit buried.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]

Re: [kernel.org users] What is a useful way to show changes between series?

[Jason Gunthorpe]

On Wed, Apr 29, 2020 at 06:50:49PM +0100, Mark Brown wrote:
> On Wed, Apr 29, 2020 at 10:42:01AM -0700, Junio C Hamano wrote:
> 
> > Are we talking about format.useAutoBase or something more elaborate?
> 
> Ah, indeed I think that is exactly what I'm looking for!  Sadly it is
> not documented in the git format-patch man page so I hadn't seen it,

So if we use this, should we set the --base to be a linux-next tag?

Is that reliable enough?

Can't relly use Linus's tree, and if people randomly choose their
upstream's then it defeats the whole point :(

> even the base auto option is a bit buried.

Agree, that seems to be missed from the synopsis and option
description sections :)

Jason

Re: [kernel.org users] What is a useful way to show changes between series?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

On Wed, Apr 29, 2020 at 02:58:44PM -0300, Jason Gunthorpe wrote:
> On Wed, Apr 29, 2020 at 06:50:49PM +0100, Mark Brown wrote:
> > On Wed, Apr 29, 2020 at 10:42:01AM -0700, Junio C Hamano wrote:

> > Ah, indeed I think that is exactly what I'm looking for!  Sadly it is
> > not documented in the git format-patch man page so I hadn't seen it,

> So if we use this, should we set the --base to be a linux-next tag?

> Is that reliable enough?

> Can't relly use Linus's tree, and if people randomly choose their
> upstream's then it defeats the whole point :(

Personally I'd just not bother specifying a base if it's -next or
there's platform enablement stuff in there - I do send a bunch of stuff
that's based off directly off a rc or something in a maintainer's tree
so for me it's quite useful.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]

Re: [kernel.org users] What is a useful way to show changes between series?

[Jason Gunthorpe]

On Wed, Apr 29, 2020 at 07:09:41PM +0100, Mark Brown wrote:
> On Wed, Apr 29, 2020 at 02:58:44PM -0300, Jason Gunthorpe wrote:
> > On Wed, Apr 29, 2020 at 06:50:49PM +0100, Mark Brown wrote:
> > > On Wed, Apr 29, 2020 at 10:42:01AM -0700, Junio C Hamano wrote:
> 
> > > Ah, indeed I think that is exactly what I'm looking for!  Sadly it is
> > > not documented in the git format-patch man page so I hadn't seen it,
> 
> > So if we use this, should we set the --base to be a linux-next tag?
> 
> > Is that reliable enough?
> 
> > Can't relly use Linus's tree, and if people randomly choose their
> > upstream's then it defeats the whole point :(
> 
> Personally I'd just not bother specifying a base if it's -next or
> there's platform enablement stuff in there - I do send a bunch of stuff
> that's based off directly off a rc or something in a maintainer's tree
> so for me it's quite useful.

I mean, if I use --base to be linux-next then git format-patch will
find some common near commit that is easy for tooling to access.

eg if my series has netdev as my base then --base=linux-next will pull
out the latest netdev tree (I assume it usese git merge-base
internally?). Same for basically all the subsystem trees in the
kernel..

I'm not suggesting to send patches against linux-next

Jason

Re: [kernel.org users] What is a useful way to show changes between series?

[Konstantin Ryabitsev]

On Wed, Apr 29, 2020 at 02:58:44PM -0300, Jason Gunthorpe wrote:
> > Ah, indeed I think that is exactly what I'm looking for!  Sadly it 
> > is
> > not documented in the git format-patch man page so I hadn't seen it,
> 
> So if we use this, should we set the --base to be a linux-next tag?
> 
> Is that reliable enough?
> 
> Can't relly use Linus's tree, and if people randomly choose their
> upstream's then it defeats the whole point :(

One option I can think of is for git.kernel.org to provide a lookup API 
that would consume the commit-id and spit out the tree and branch where 
this object exists, working down the list of preferred trees:

1. torvalds/linux.git
2. next/linux-next.git
3. stable/linux.git
4. everyone else on kernel.org
5. select remote trees that we can start mirroring

This should help make this information a lot more useful for automation, 
I expect.

-K

Re: [kernel.org users] What is a useful way to show changes between series?

[mpe]

"Konstantin Ryabitsev" <konstantin@linuxfoundation.org> writes:
> Hi, all:
>
> What would be a useful way to quickly do a "show me what changed" 
> between two versions of the same patch series? Especially considering 
> that:
>
> - there may be a different number of patches in the series
> - new revisions will likely be based on newer trees
> - subjects/commit messages/trailers may be vastly different between two 
>   series, even if the code is very similar (e.g. someone broke down a 
>   single large commit into several smaller ones or vice-versa)
>
> In my mind, the best way to approach this would be:
>
> 1. Create a single unified diff file from all patches in each series 
>    (using combinediff)
> 2. Remove all context, leaving only -/+ lines (using interdiff -U0)
> 3. Remove all line numbers from @@ @@ hunks (replacing them with 0)
> 4. Add commit information on top of the diff
>
> In the end, we get two summary documents that can be compared using 
> regular diffing tools like vimdiff/meld/etc and actually show a fairly 
> succinct "how are these two series different" view.
>
> The goal is not to replace code review, obviously, but to give a way for 
> a maintainer to do a quick sanity-check of a series they've reviewed 
> before.
>
> I ran this scenario on a random patch series that I found 
> (https://lore.kernel.org/lkml/b2b9cb06-b0eb-3e94-b83a-e58467069ae2@gmail.com/) 
> and attached are two summary files:
>
>  - phy_v2.summary
>  - phy_v3.summary
>
> Here's how diffing them looks in vimdiff and meld:
>
>  - https://mricon.com/misc/phy-summary-vimdiff-fs8.png
>  - https://mricon.com/misc/phy-summary-meld-fs8.png
>
> So, questions for you are:
>
> 1. Is this is a useful way to present an "at-a-glance" view?

Yes, I think in a lot of cases it would be.

It's fairly common I think for a series to not change a lot in later
versions. So eg. the series might be 10 patches with 1000 lines changed
total but a new revision might only change 10 lines. For cases like that
this would be very useful compared to trawling through 10 patches
individually.

And even for a larger delta between the series I think this will often
work quite well, as long as there isn't too much code movement.

Where this will fall down is a series that is undergoing large sweeping
changes, but that case is just plain hard to visualise well.

> 2. If yes, how would you change it to be more useful?

There will be cases where the diff of the whole series is going to be
too big and/or busy. So having a way to drill down would be awesome.

The easiest to implement would be limiting it to a single file, which
would be useful in some cases.

Harder to implement but possibly more useful would be showing the diff
between the two series up to some point, so eg. show me the changes
between the series but only patches 1-5.

cheers

Re: [kernel.org users] What is a useful way to show changes between series?

[Paolo Bonzini]

On 29/04/20 06:16, Michael Ellerman wrote:
> 
> What would be a useful way to quickly do a "show me what changed" 
> between two versions of the same patch series? Especially considering 
> that:
> 
> - there may be a different number of patches in the series
> - new revisions will likely be based on newer trees
> - subjects/commit messages/trailers may be vastly different between two 
>   series, even if the code is very similar (e.g. someone broke down a 
>   single large commit into several smaller ones or vice-versa)
> 
> In my mind, the best way to approach this would be:
> 
> 1. Create a single unified diff file from all patches in each series 
>    (using combinediff)
> 2. Remove all context, leaving only -/+ lines (using interdiff -U0)
> 3. Remove all line numbers from @@ @@ hunks (replacing them with 0)
> 4. Add commit information on top of the diff

You have to do what "git range-diff" does:

- Remove all line numbers from @@ @@ lines

- Remove commit message too if you want, though that's not strictly necessary
(sometimes small patches can change a lot but commit messages stay the same)

- Add dummy empty patches to the shorter series to detect new or deleted patches

- Compute diff of all pairs of diffs (this can be optimized by looking in advance
for completely equal patches - maybe removing the commit message would help for
finding equal patches)

- Compute a "weight" that measures how different the patches are (for example, number
of lines removed + number of lines inserted)

- Solve an assignment problem using the Hungarian algorithm

- Figure out the order in which to present patches (e.g. sort them 
according to the new series, including deleted patches from the previous 
version after all the patches that came before it).


You can find an implementation in Javascript of the last five steps at

https://github.com/patchew-project/patchew/blob/master/www/templates/series-diff.html

with the Hungarian algorithm at

https://github.com/patchew-project/patchew/blob/master/static/jsdifflib/munkres.js

(but there are Python implementations available in PyPI too).

Thanks,

Paolo

Re: [tools] [kernel.org users] What is a useful way to show changes between series?

[Konstantin Ryabitsev]

On Wed, 29 Apr 2020 at 00:16, Michael Ellerman <mpe@ellerman.id.au> wrote:
> There will be cases where the diff of the whole series is going to be
> too big and/or busy. So having a way to drill down would be awesome.
>
> The easiest to implement would be limiting it to a single file, which
> would be useful in some cases.

With Jason's help, I think I have a good idea how we can do this. It's
not infallible, but it moves most of the logic to git-range-diff,
which is how I prefer it anyway. I'll try to figure out how to limit
this to a subpath and if I can do it without it potentially causing
more confusion, then I'll try to implement it.

> Harder to implement but possibly more useful would be showing the diff
> between the two series up to some point, so eg. show me the changes
> between the series but only patches 1-5.

Well, the upside of using git-range-diff for the actual comparison is
that it makes it very easy to locate the commits that you're
interested in.

Let me try to put together something that folks can try out and then
we can work iteratively from there.

Thanks for everyone's input.

-K

Re: [kernel.org users] What is a useful way to show changes between series?

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]

On Tue 2020-04-28 15:29:38, Konstantin Ryabitsev wrote:
> Hi, all:
> 
> What would be a useful way to quickly do a "show me what changed" 
> between two versions of the same patch series? Especially considering 
> that:
> 
> - there may be a different number of patches in the series
> - new revisions will likely be based on newer trees
> - subjects/commit messages/trailers may be vastly different between two 
>   series, even if the code is very similar (e.g. someone broke down a 
>   single large commit into several smaller ones or vice-versa)
> 
> In my mind, the best way to approach this would be:
> 
> 1. Create a single unified diff file from all patches in each series 
>    (using combinediff)
> 2. Remove all context, leaving only -/+ lines (using interdiff -U0)
> 3. Remove all line numbers from @@ @@ hunks (replacing them with 0)
> 4. Add commit information on top of the diff

You know about interdiff utility, do you?


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 201 bytes --]