> On Jan 20, 2020, at 6:41 PM, Gary Gregory <garydgreg...@gmail.com> wrote:
>
> On Mon, Jan 20, 2020 at 6:28 PM Gary Gregory <garydgreg...@gmail.com> wrote:
>
>>> On Sun, Jan 19, 2020 at 7:39 AM Alex Herbert <alex.d.herb...@gmail.com>
>>> wrote:
>>>
>>> Hi Gary,
>>>
>>> I raised a few niggles a while back with CSV and the discussion did not
>>> receive a response on how to proceed.
>>>
>>> There is the major bug CSV-248 where the CSVRecord is not Serializable
>>> [1]. This requires a decision on what to do to fix it. This bug is still
>>> present in 1.8 RC1 as found by FindBugs [2].
>>>
>>> From what I can see the CSVRecord maintains a reference to the CSVParser.
>>> This chain of objects maintained in memory is not serializable and leads
>>> back to the original input Reader.
>>>
>>> I can see from the JApiCmp report that the serial version id was changed
>>> for CSVRecord this release so there is still an intention to support
>>> serialization. So this should be a blocker.
>>>
>>> I could not find a serialisation test in the unit tests for CSVRecord.
>>> This quick test added to CSVRecordTest fails:
>>>
>>>
>>> @Test
>>> public void testSerialization() throws IOException {
>>> CSVRecord shortRec;
>>> try (final CSVParser parser = CSVParser.parse("a,b",
>>> CSVFormat.newFormat(','))) {
>>> shortRec = parser.iterator().next();
>>> }
>>> final ByteArrayOutputStream out = new ByteArrayOutputStream();
>>> try (ObjectOutputStream oos = new ObjectOutputStream(out)) {
>>> oos.writeObject(shortRec);
>>> }
>>> }
>>>
>>> mvn test -Dtest=CSVRecordTest
>>>
>>> [ERROR] testSerialization Time elapsed: 0.032 s <<< ERROR!
>>> java.io.NotSerializableException: org.apache.commons.csv.CSVParser
>>> at
>>> org.apache.commons.csv.CSVRecordTest.testSerialization(CSVRecordTest.java:235)
>>>
>>> If I mark the field csvParser as transient it passes. So this is a
>>> problem as raised by FindBugs.
>>>
>>
>> Making the field transient would indeed fix this test but... some of the
>> record APIs would then fail with NPEs... so we would be kicking the can
>> down the road basically. Making the parser serializable is going in the
>> wrong direction feature-wise IMO, so let's not go there. A serialization
>> proxy would be less worse but should we provide such a thing? I would say
>> no. I am OK with making the field transient despite the can kicking, so
>> let's do that.
>>
>
> I suppose we should bump the serialVersionUID...
+1 to that
> Gary
>
>
>>
>> Gary
>>
>>
>>>
>>>
>>> I also raised [3] the strange implementation of the CSVParser
>>> getHeaderNames() which ignores null headers as they cannot be used as a key
>>> into the map. However the list of column names could contain the null
>>> values. This test currently fails:
>>>
>>> @Test
>>> public void testHeaderNamesWithNull() throws IOException {
>>> final Reader in = new
>>> StringReader("header1,null,header3\n1,2,3\n4,5,6");
>>> final Iterator<CSVRecord> records = CSVFormat.DEFAULT.withHeader()
>>>
>>> .withNullString("null")
>>>
>>> .withAllowMissingColumnNames()
>>>
>>> .parse(in).iterator();
>>> final CSVRecord record = records.next();
>>> assertEquals(Arrays.asList("header1", null, "header3"),
>>> record.getParser().getHeaderNames());
>>> }
>>>
>>> I am not saying it should pass but at least the documentation should
>>> state the behaviour in this edge case. That is the list of header names may
>>> be shorter than the number of columns when the parser is configured to
>>> allow null headers. I’ve not raised a bug ticket for this as it is open to
>>> opinion if this is by design or actually a bug. This issue is still present
>>> in 1.8 RC1.
>>>
>>> Previously I suggested documentation changes for this and another edge
>>> case using the header map to be added to the javadoc for getHeaderNames()
>>> and getHeaderMap():
>>>
>>> - Documentation:
>>>
>>> The mapping is only guaranteed to be a one-to-one mapping if the record
>>> was created with a format that does not allow duplicate or null header
>>> names. Null headers are excluded from the map and duplicates can only map
>>> to 1 column.
>>>
>>>
>>> - Bug / Documentation
>>>
>>> The CSVParser only stores headers names in a list of header names if they
>>> are not null. So the list can be shorter than the number of columns if you
>>> use a format that allows empty headers and contains null column names.
>>>
>>>
>>> The ultimate result is that we should document that the purpose of the
>>> header names is to provide a list of non-null header names in the order
>>> they occur in the header and thus represent keys that can be used in the
>>> header map. In certain circumstances there may be more columns in the data
>>> than there are header names.
>>>
>>>
>>> Alex
>>>
>>>
>>> [1] https://issues.apache.org/jira/browse/CSV-248 <
>>> https://issues.apache.org/jira/browse/CSV-248>
>>>
>>> [2]
>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/findbugs.html
>>> <
>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/findbugs.html
>>>>
>>>
>>> [3] https://markmail.org/message/woti2iymecosihx6 <
>>> https://markmail.org/message/woti2iymecosihx6>
>>>
>>>
>>>
>>>> On 18 Jan 2020, at 17:52, Gary Gregory <ggreg...@apache.org> wrote:
>>>>
>>>> We have fixed quite a few bugs and added some significant enhancements
>>>> since Apache Commons CSV 1.7 was released, so I would like to release
>>>> Apache Commons CSV 1.8.
>>>>
>>>> Apache Commons CSV 1.8 RC1 is available for review here:
>>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1 (svn
>>>> revision 37670)
>>>>
>>>> The Git tag commons-csv-1.8-RC1 commit for this RC is
>>>> c1c8b32809df295423fc897eae0e8b22bfadfe27 which you can browse here:
>>>>
>>>>
>>> https://gitbox.apache.org/repos/asf?p=commons-csv.git;a=commit;h=c1c8b32809df295423fc897eae0e8b22bfadfe27
>>>> You may checkout this tag using:
>>>> git clone https://gitbox.apache.org/repos/asf/commons-csv.git
>>> --branch
>>>> commons-csv-1.8-RC1 commons-csv-1.8-RC1
>>>>
>>>> Maven artifacts are here:
>>>>
>>>>
>>> https://repository.apache.org/content/repositories/orgapachecommons-1489/org/apache/commons/commons-csv/1.8/
>>>>
>>>> These are the artifacts and their hashes:
>>>>
>>>> #Release SHA-512s
>>>> #Sat Jan 18 12:01:01 EST 2020
>>>>
>>> commons-csv-1.8-bin.tar.gz=85a876b41aa9ce61f7f533c46df48754e05bddbdef892aed2bac7674b5ea13855de25576364649048dbb55e7fb18a354305b56cb697e85df68a87113954128ed
>>>>
>>> commons-csv-1.8-bin.zip=9b86a22367c84a0c96a457e8495f81113b64ae5501eabbe2ea4137654b6baa05bcc24a19626452b80e30ff2dd39214840c6ec534be1db9eec2d12c93eeab2de1
>>>>
>>> commons-csv-1.8-javadoc.jar=a481149dfeffe4e915d5d2e846831994223fc7d09ed2b61398c68eed5a672654a141fa6de705aa743d0b5af6fd24a3f4b0d5e7cee238a1f7642673288d4a985d
>>>>
>>> commons-csv-1.8-sources.jar=f68e50f8a025a8b2a570b46905b22b5753a83c19bee5c38103d92ec1e47b4e0d27353e7931961e74fe8e67c4909b0ece6ede49a585d2f9180a7a15458b020bc0
>>>>
>>> commons-csv-1.8-src.tar.gz=c3268f456978e75c19134e35d05bff77002b2fa7439be2623d58a102cab4f93b0913a1a789f962aafcd677938be1547f47c5dd86e3ea08b7bf8f0420e81beb7a
>>>>
>>> commons-csv-1.8-src.zip=ebb32f2406b6afa48472283612c7d0a94f932d7ae7a72ad1d239e2249de12f1e0da7f61d34d95d66b1d1fe95b66b6316af9d1fc93734f610cce4a7163b0900d0
>>>>
>>> commons-csv-1.8-test-sources.jar=13508d417e23a5d3f575a39b3aedb20d0d834335d7994f3045fff316e6b12e50cbf9afe908271357b4091d981c178a28dc61bcdb8db60bd0ada07d3de59eacbf
>>>>
>>> commons-csv-1.8-tests.jar=901889d4be203c2044df89b7e051d21e7b806e5e56438bf9a7483b334331da94b71de1a129c8bf7967e02479a0922bb834ce37eaabf6662702e147813ecb2b7f
>>>>
>>>> I have tested this with ' mvn -V -Prelease -Ptest-deploy -P jacoco -P
>>>> japicmp clean package site deploy' using:
>>>>
>>>> Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
>>>> Maven home: C:\Java\apache-maven-3.6.3\bin\..
>>>> Java version: 1.8.0_241, vendor: Oracle Corporation, runtime: C:\Program
>>>> Files\Java\jdk1.8.0_241\jre
>>>> Default locale: en_US, platform encoding: Cp1252
>>>> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
>>>>
>>>> Additional tests with 'mvn -V clean test' using:
>>>>
>>>> Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
>>>> Maven home: C:\Java\apache-maven-3.6.3\bin\..
>>>> Java version: 1.8.0_241, vendor: Oracle Corporation, runtime: C:\Program
>>>> Files\Java\jdk1.8.0_241\jre
>>>> Default locale: en_US, platform encoding: Cp1252
>>>> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
>>>>
>>>> Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
>>>> Maven home: C:\Java\apache-maven-3.6.3\bin\..
>>>> Java version: 11.0.6, vendor: Oracle Corporation, runtime: C:\Program
>>>> Files\Java\jdk-11.0.6
>>>> Default locale: en_US, platform encoding: Cp1252
>>>> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
>>>>
>>>> Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
>>>> Maven home: C:\Java\apache-maven-3.6.3\bin\..
>>>> Java version: 12.0.2, vendor: Oracle Corporation, runtime: C:\Program
>>>> Files\Java\jdk-12.0.2
>>>> Default locale: en_US, platform encoding: Cp1252
>>>> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
>>>>
>>>> Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
>>>> Maven home: C:\Java\apache-maven-3.6.3\bin\..
>>>> Java version: 13.0.2, vendor: Oracle Corporation, runtime: C:\Program
>>>> Files\Java\jdk-13.0.2
>>>> Default locale: en_US, platform encoding: Cp1252
>>>> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
>>>>
>>>> Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
>>>> Maven home: C:\Java\apache-maven-3.6.3\bin\..
>>>> Java version: 14-ea, vendor: Oracle Corporation, runtime: C:\Program
>>>> Files\Java\openjdk\jdk-14
>>>> Default locale: en_US, platform encoding: Cp1252
>>>> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
>>>>
>>>> JaCoCo fails on Java 15-EA because it does not know about class file
>>> major
>>>> version 59:
>>>>
>>>> Caused by: java.lang.IllegalArgumentException: Unsupported class file
>>> major
>>>> version 59
>>>> at
>>>>
>>> org.jacoco.agent.rt.internal_43f5073.asm.ClassReader.<init>(ClassReader.java:195)
>>>>
>>>> Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
>>>> Maven home: C:\Java\apache-maven-3.6.3\bin\..
>>>> Java version: 15-ea, vendor: Oracle Corporation, runtime: C:\Program
>>>> Files\Java\openjdk\jdk-15
>>>> Default locale: en_US, platform encoding: Cp1252
>>>> OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
>>>>
>>>> Details of changes since 1.7 are in the release notes:
>>>>
>>>>
>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/RELEASE-NOTES.txt
>>>>
>>>>
>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/changes-report.html
>>>>
>>>> Site:
>>>>
>>>>
>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/index.html
>>>> (note some *relative* links are broken and the 1.8 directories are
>>> not
>>>> yet created - these will be OK once the site is deployed.)
>>>>
>>>> JApiCmp Report (compared to 1.7):
>>>>
>>>>
>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/japicmp.html
>>>>
>>>> RAT Report:
>>>>
>>>>
>>> https://dist.apache.org/repos/dist/dev/commons/csv/1.8-RC1/site/rat-report.html
>>>>
>>>> KEYS:
>>>> https://www.apache.org/dist/commons/KEYS
>>>>
>>>> Please review the release candidate and vote.
>>>> This vote will close no sooner that 72 hours from now.
>>>>
>>>> [ ] +1 Release these artifacts
>>>> [ ] +0 OK, but...
>>>> [ ] -0 OK, but really should fix...
>>>> [ ] -1 I oppose this release because...
>>>>
>>>> Thank you,
>>>>
>>>> Gary Gregory,
>>>> Release Manager (using key 86fdc7e2a11262cb)
>>>>
>>>> For following is intended as a helper and refresher for reviewers.
>>>>
>>>> Validating a release candidate
>>>> ==============================
>>>>
>>>> These guidelines are NOT complete.
>>>>
>>>> Requirements: Git, Java, Maven.
>>>>
>>>> You can validate a release from a release candidate (RC) tag as follows.
>>>>
>>>> 1) Clone and checkout the RC tag
>>>>
>>>> git clone https://gitbox.apache.org/repos/asf/commons-csv.git --branch
>>>> commons-csv-1.8-RC1 commons-csv-1.8-RC1
>>>> cd commons-csv-1.8-RC1
>>>>
>>>> 2) Check Apache licenses
>>>>
>>>> This step is not required if the site includes a RAT report page which
>>> you
>>>> then must check.
>>>>
>>>> mvn apache-rat:check
>>>>
>>>> 3) Check binary compatibility
>>>>
>>>> Older components still use Apache Clirr:
>>>>
>>>> This step is not required if the site includes a Clirr report page which
>>>> you then must check.
>>>>
>>>> mvn clirr:check
>>>>
>>>> Newer components use JApiCmp with the japicmp Maven Profile:
>>>>
>>>> This step is not required if the site includes a JApiCmp report page
>>> which
>>>> you then must check.
>>>>
>>>> mvn install -DskipTests -P japicmp japicmp:cmp
>>>>
>>>> 4) Build the package
>>>>
>>>> mvn -V clean package
>>>>
>>>> You can record the Maven and Java version produced by -V in your VOTE
>>> reply.
>>>> To gather OS information from a command line:
>>>> Windows: ver
>>>> Linux: uname -a
>>>>
>>>> 5) Build the site for a single module project
>>>>
>>>> Note: Some plugins require the components to be installed instead of
>>>> packaged.
>>>>
>>>> mvn site
>>>> Check the site reports in:
>>>> - Windows: target\site\index.html
>>>> - Linux: target/site/index.html
>>>>
>>>> 6) Build the site for a multi-module project
>>>>
>>>> mvn site
>>>> mvn site:stage
>>>> Check the site reports in:
>>>> - Windows: target\site\index.html
>>>> - Linux: target/site/index.html
>>>>
>>>> -the end-
>>>
>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
