[MGNLUI-34] Streamline VMagnoliaTable: use mvn patch to make it maintainable Created: 12/Sep/12  Updated: 11/Feb/13  Resolved: 09/Nov/12

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: None
Fix Version/s: 5.0

Type: Task Priority: Critical
Reporter: Daniel Lipp Assignee: Samuli Penttilä
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
Template:
Acceptance criteria:
Empty
Task DoR:
Empty
Date of First Response:

 Description   

VMagnoliaTable (around 7000 loc) is basically a modified copy of Vaadin's VScrollTable with a bunch of todo's & fixme's as well as inconsistent javadoc. We should check for a way to not have to maintain it by ourselves (e.g. by getting VScrollTable adapted in a way we can simply subclass it and overwrite only what needs to be different).



 Comments   
Comment by Federico Grilli [ 19/Oct/12 ]

I am trying to use the following Maven-based approach

  • extract Vaadin's VScrollTable sources
  • patch it

This is the relevant pom snippet

<build>
...
<plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-dependency-plugin</artifactId>
        <executions>
          <execution>
            <id>unpack-vaadin-treetable-src</id>
            <phase>process-sources</phase>
            <goals>
              <goal>unpack</goal>
            </goals>
            <configuration>
              <artifactItems>
                <artifactItem>
                  <groupId>com.vaadin</groupId>
                  <artifactId>vaadin</artifactId>
                  <version>${vaadinVersion}</version>
                  <type>jar</type>
                  <classifier>sources</classifier>
                  <overWrite>true</overWrite>
                  <outputDirectory>src/main/java</outputDirectory>
                </artifactItem>
              </artifactItems>
             <includes>**\/*VScrollTable.java</includes>
            </configuration>
          </execution>
        </executions>
      </plugin>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-patch-plugin</artifactId>
        <version>1.1.1</version>
        <configuration>
          <patches>
            <patch>vscrolltable.patch</patch>
          </patches>
         <targetDirectory>src/main/java</targetDirectory>
        </configuration>
        <executions>
          <execution>
            <id>patch</id>
            <goals>
              <goal>apply</goal>
            </goals>
          </execution>
        </executions>
      </plugin>

Then I have this simple patch generated with git diff -p where I'm trying to make a private field protected. If you are unfamiliar with the patch format and in particular the line starting and ending with @@ it basically means that line #130 in VScrollTable.java has to become the same as the line preceded by the plus sign. The second number here (1) means the number of lines to be change before applying the patch and the number of changed lines after the patch is applied.

--- com/vaadin/terminal/gwt/client/ui/VScrollTable.java
+++ com/vaadin/terminal/gwt/client/ui/VScrollTable.java
@@ -130,1 +130,1 @@
-    private static final String ROW_HEADER_COLUMN_KEY = "0";
+    protected static final String ROW_HEADER_COLUMN_KEY = "0";

The source unpacking works fine, the maven-patch-plugin actually tries to apply the patch but, alas!, with no luck as you can see in the snippet below
the result of running mvn -X clean install. Maybe the failure has to do with line endings differences between Vaadin source and how I save this file in my local git repo (see http://stackoverflow.com/questions/1967370/git-replacing-lf-with-crlf)

[DEBUG] Configuring mojo 'org.apache.maven.plugins:maven-patch-plugin:1.1.1:apply' -->
[DEBUG]   (f) backups = false
[DEBUG]   (f) failFast = true
[DEBUG]   (f) ignoreWhitespace = true
[DEBUG]   (f) naturalOrderProcessing = false
[DEBUG]   (f) optimizations = true
[DEBUG]   (f) patchDirectory = /Users/fgrilli/devel/workspace/magnolia/magnolia_ui/magnolia-ui-vaadin-widgetset/src/main/patches
[DEBUG]   (f) patchTrackingFile = /Users/fgrilli/devel/workspace/magnolia/magnolia_ui/magnolia-ui-vaadin-widgetset/target/optimization-files/patches-applied.txt
[DEBUG]   (f) patches = [vscrolltable.patch]
[DEBUG]   (f) removeEmptyFiles = false
[DEBUG]   (f) reverse = false
[DEBUG]   (f) skipApplication = false
[DEBUG]   (f) strictPatching = false
[DEBUG]   (f) strip = 0
[DEBUG]   (f) targetDirectory = /Users/fgrilli/devel/workspace/magnolia/magnolia_ui/magnolia-ui-vaadin-widgetset/src/main/java
[DEBUG]   (f) useDefaultIgnores = true
[DEBUG] -- end configuration --
[INFO] [patch:apply {execution: patch}]
[DEBUG] Looking for patch: vscrolltable.patch in: /Users/fgrilli/devel/workspace/magnolia/magnolia_ui/magnolia-ui-vaadin-widgetset/src/main/patches/vscrolltable.patch
[INFO] Applying patch: vscrolltable.patch
[DEBUG] Executing: /bin/sh -c cd /Users/fgrilli/devel/workspace/magnolia/magnolia_ui/magnolia-ui-vaadin-widgetset/src/main/java && patch -p0 -l -i /Users/fgrilli/devel/workspace/magnolia/magnolia_ui/magnolia-ui-vaadin-widgetset/src/main/patches/vscrolltable.patch
[DEBUG] patching file com/vaadin/terminal/gwt/client/ui/VScrollTable.java
[DEBUG] Hunk #1 FAILED at 130.
[DEBUG] 1 out of 1 hunk FAILED -- saving rejects to file com/vaadin/terminal/gwt/client/ui/VScrollTable.java.rej
[DEBUG] Exit code: 1
[INFO] ------------------------------------------------------------------------
[ERROR] BUILD ERROR
[INFO] ------------------------------------------------------------------------

Comment by Federico Grilli [ 23/Oct/12 ]

After having set the Maven-based process to extract, patch and rename Vaadin's VScrollTable we need the actual patch to be applied. The Maven patching process is still isolated in a profile not active by default, which can be triggered simply by mvn clean install -Ppatch-vaadin-vscrolltable. The reason is that we rename the original file to VScrollTablePatched while its contents remain the same as the original, hence compilation errors due to a mismatch between file name and classname. Once we will have the actual patch dealing with it too, the profile can be made active by default or we can just get rid of it and put the plugins used in the patching process in the normal Maven build cycle.
Reassigning this to Sasha who can create the patch and refactor VMagnoliaTable accordingly.

Comment by Samuli Penttilä [ 09/Nov/12 ]

Patch modifies private fields to protected. Also because of nested classes object construction is put to factory methods that are overridable. Indexing changes are tackled by not to insert extra column for checkboxes. Instead checkboxes are inserted inside first column cells.

Future improvement: Instead of static patch, use script that transforms field visibility and replaces 'new Object()' to 'createObject()' method. Static patch may become obsolete at every version update of VScrollTable.

Generated at Mon Feb 12 08:32:56 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.