Community
Participate
Working Groups
We realized earlier today that the artifact MirrorApplication doesn't simply mirror the files but also performs the processing steps. This doesn't really affect canonical artifacts but causes packed files to be downloaded and then unpacked (undesirable). This bug is blocked by Bug 249994.
Created attachment 114472 [details] Lets Mirroring bypass processing steps Added a getRawArtifact(IArtifactDescriptor) method to SimpleArtifactRepository. Basically it's the exact same as getArtifact(IArtifactDescriptor) except it removes all code related to processing steps. I didn't modify getArtifact since getRawArtifact currently amounts to a single line. Note: the getRawArtifact call in Mirroring.java currently requires a cast to SimpleArtifactRepository (until the API is modified) Tested using the MirrorApplication test cases and verified the resulting files manually.
This new method needs to be added on the IArtifactRepository
I thought I said something about that in my last comment. Sorry. It currently requires casting which will need to be removed. I can add it to IArtifactRepository myself but ti also requires me changing all the dependent files (4, I think). I'll look into it.
Created attachment 114540 [details] Modifies IArtifactReposiroty, addeds require methods. Same as first patch except I added the getRawArtifact method to IArtifactRepository and all classes the implement that interface. I removed the casting from the Mirroring class. I got some failures in org.eclipse.equinox.p2.tests.reconciler.dropins.AllTests but I'm not sure that was my faults. Either way I don't have the set up to test the changes I've made so please make sure I didn't break anything.
In the light of the changes on TestArtifactRepository#getRawArtifact(), the implementation of TestArtifactRepository#getArtifact() needs to be reviewed since it does not honor the API. AbstractProvisioningTest does not belong to this patch. We also need to have a test checking that for two SAR the files are of the same size in each repo.
Created attachment 114593 [details] Revised patch Fixed up the test case and removed the unnecessary changes. Requires test data I attached below
Created attachment 114594 [details] test data for patch Must be placed in the testData/mirror folder.
I have released the changes in the application and the new API method. However I have not released the tests because they were failing. I identified two problems: - the assertContains method seems to assume that the descriptors have to be provided in the same order where it is not necessary - the artifact descriptor in the source and the target repo are not equals. The properties related to size are not equals. Of course the fundamental issue is around the identity of the artifact descriptor, however I find worrying that the artifact descriptor does not have the correct values. You may want to open a bug report to capture this.
Created attachment 114791 [details] Fixes test cases I (hopefully) fixed the issues you were having. I included the temporary asserts I was using to verify descriptors in the AbstractProvisionTest. We should look into changing them when that issue gets resolved. Also this patch includes some mirror changes that fix some problems I was having after we changed from URLs to URIs. This patch still requires the test data I attached earlier.
Created attachment 115034 [details] Updated fixed test cases No changes from the previous version, just synchronized and recreated the patch.
I have released the tests. Closing. Andrew did you verify why the size of the artifact in the descriptor was different in the source and the target.
closing for real
(In reply to comment #11) > I have released the tests. Closing. > Andrew did you verify why the size of the artifact in the descriptor was > different in the source and the target. > I thought you discovered it was due to republishing artifacts with minor changes but with out changing their version number.
