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

JBDS-3929 apply the same content.xml... #56

Open
wants to merge 3 commits into
base: jbosstools-4.4.0.x
Choose a base branch
from

Conversation

nickboldt
Copy link
Member

JBDS-3929 apply the same content.xml transformations in content.jar as in content.xml.xz

@mickaelistria
Copy link
Contributor

Didn't try it and cannot do it right now, but it seems fine in principle.

@nickboldt
Copy link
Member Author

nickboldt commented Jun 7, 2016

PR revised. Need to do the same transform for artifacts.* too. Also refactored some variables.

…s in content.xml.xz

JBDS-3929 apply the same artifacts.xml transformations in artifacts.jar as in artifacts.xml.xz
// see also https://bugs.eclipse.org/bugs/show_bug.cgi?id=464614
//getLog().debug("delete old artifacts.xml.xz");
File artifactsXmlXz = new File(p2repository,"artifacts.xml.xz");
FileUtils.forceDelete(artifactsXmlXz);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this code for .xz'en the file be put in a separate method instead of duplicating that many lines ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thx.

@maxandersen
Copy link
Member

I can't test it but the intent is fine but I would from the start avoid the big duplication of the code to do the .xz compression.

+1 when thats cleaned up.

while (-1 != (n = in.read(buffer))) {
out.write(buffer, 0, n);
}
out.close();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no try/catch or similar to report to user what file possibly failed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extracted method alterXzFile throws IOException, FileNotFoundException, TransformerException ... is that enough or do you still need try/catch blocks to trap for these errors and dump more details to console?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neither of those exceptions will actually tell you what file the issue is with. I would recommend catching them and rethrowing with them nested inside a Tycho or Maven based runtime exception that will have a message that will help you actually track down what is the problem.

@nickboldt
Copy link
Member Author

BTW to test this, just mvn clean install it locally, then go into jbdevstudio-product/ from master branch and mvn clean install there too. Results are in site/target/repository, which you can then browse from Eclipse or JBDS to see what's available in the update site.

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.

4 participants