git-plugin icon indicating copy to clipboard operation
git-plugin copied to clipboard

Published 20 hours ago • verified publisher icon jenkinsci

Make GitChangeSet serializable (for use in Jenkins Pipeline CPS methods)

Open alexdowad opened this issue 4 years ago • 3 comments
trafficstars

Please let me know if there is any reason why this change doesn't make sense! I would have opened a GH 'issue' first for discussion, but it seems that Issues are not enabled on this repository...

Commit message:

"Jenkins requires that objects manipulated in Jenkins Pipeline Groovy methods which are not marked @NoCPS must be serializable. It seems that there is no particular reason why GitChangeSet should not be serializable, so mark it as such."

Checklist

  • [x] I have read the CONTRIBUTING doc
  • [ ] I have added tests that verify my changes
  • [ ] Unit tests pass locally with my changes
  • [ ] No Javadoc warnings were introduced with my changes
  • [ ] No spotbugs warnings were introduced with my changes
  • [ ] Online help has been added and reviewed for any new or modified fields
  • [ ] I have interactively tested my changes

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)

alexdowad avatar Jun 04 '21 11:06 alexdowad

https://issues.jenkins.io/browse/JENKINS-65881 indicates that GitChangeSet cannot be serialized by the Jenkins serialization system because it includes a field that is known to be unsafe for serialization.

MarkEWaite avatar Jun 12 '21 20:06 MarkEWaite

@MarkEWaite Thanks so much for the response!! And thanks for the link to the Jenkins bug tracker!

The issue you linked to indicates that the field in GitChangeSet which can't be serialized is dateFormatters, which contains three DateTimeFormatter objects. I'm wondering about that, though... it seems that every single GitChangeSet object contains three equivalent DateTimeFormatters. (They are initialized in the constructor, but the initialization code doesn't use the parameters passed into the constructor at all, so those objects should always be "the same". I wonder if they could even be shared as class fields rather than instance fields. Or else custom serialization methods could be defined which wouldn't serialize those fields out; again, the values are always the same, so there is actually no need to serialize and deserialize them.)

Does that make sense?? I am not familiar with this codebase, so I could easily be totally off base here.

alexdowad avatar Jun 12 '21 21:06 alexdowad

Looks like @MarkEWaite added the code with those DateTimeFormatters in b84c7c9dcea3a30a87eadb7d20c8be773cab46e3.

alexdowad avatar Jun 13 '21 17:06 alexdowad