Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "zeroEntryTimestamps()" option #246

Closed
wants to merge 2 commits into from

Conversation

blendmaster
Copy link

See javadoc on the method for details on why and how. This
is basically the half-assed deterministic build from #229,
which is to say still extremely helpful when dealing with rsync.

Also update docs.gradle since doclet resolution freezes the build until the http request times out.

See javadoc on the method for details on why and how. This
is basically the half-assed deterministic build from GradleUp#229,
which is to say still extremely helpful when dealing with rsync.
@johnrengelman
Copy link
Collaborator

@blendmaster thanks for the submissions. I'm still finding time in my schedule to invest into some updates for shadow. As for this, I'm considering making it a higher level feature that allows a user to register arbitrary actions against each ZipEntry is is produced instead of a boolean flag.
That would make it more extensible to add new capabilities.

So I'm envisioning something like adding the following method to the ShadowJar class:

interface ZipEntryAction extends Action<ZipEntry> {}

ShadowJar zipEntries(ZipEntryAction action) {
  this.zipEntryActions << action
}

these would be passed down to the ShadowCopy Action and then applied to each ZipEntry.

If you want to take a crack at implementing it that way, that would be cool, otherwise when I find time, i'll probably incorporate your changes that way.

@blendmaster
Copy link
Author

It seems reasonable to offer more general hooks into the final shadow zip file creation. However, I'm having trouble thinking of what anybody would do with just the ZipEntry besides zero the timestamps. Did you have anything particular in mind?

If you could somehow affect both the ZipEntry and the actual contents of the entry, it might be more useful. Something like (ZipEntry e, OutputStream original) -> OutputStream transformed, where you can either wrap the original ZipOutputStream in a transform of some sort, or return it verbatim. Contrived example:

zipEntryActions << { entry, orig ->
  if (entry.name ~= "*.java") {
    // prefix text with copyright
    def written = false
    return new FilterOutputStream(orig) {
      def write(bytes) {
        if (!written) {
          orig.write("my copyright notice\n")
          written = true
        }
        orig.write(bytes)
    }
  } else {
    return orig
  }
}

In any case, I can reimplement this PR on ZipEntryAction. I'm inclined to keep the zeroEntryTimestamps() option on ShadowJar as a useful shortcut, akin to mergeServiceFiles().

@johnrengelman
Copy link
Collaborator

yeah, it might end up being something like that or even extending the Transfomer interface to also take the ZipEntry as an arg...not sure. But i think implementing it this way will provide greater flexibility for moving toward something like that.

the shortcut method is fine.

Thanks.

@blendmaster
Copy link
Author

Looking at this again, my implementation doesn't quite work with transformed (through Transformer#modifyOutputStream(ZipOutputStream)) files. Those use ZipOutputStream.putNextEntry themselves, avoiding the hook in ShadowCopyAction.

One implementation path I originally went down was to proxy the entire ZipOutputStream with something like javassist, allowing interception of all putNextEntry calls. However, I didn't want to add an extra dependency, so I went with the manual putNextEntry interception that seemed to (but doesn't) cover all callsites.

Given #247 and this PR, it does seem like a general way for munging the ZipOutputStream is appropriate. Let me know if you have any additional thoughts. I'll think of something to implement in the meantime.

@johnrengelman johnrengelman added this to the 3.0.0 milestone Nov 18, 2016
@johkelly johkelly mentioned this pull request Oct 25, 2017
@johnrengelman
Copy link
Collaborator

Closing this in favor of #333

@johnrengelman johnrengelman removed this from the 5.0.0 milestone Jan 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants