Community
Participate
Working Groups
The <p2.mirror> task has an attribute ignoreErrors which is supposed to ignore any errors that occur and allow the build to continue. When using a comparator, and compare differences are considered errors, so builds that use comparators can expect errors here and will generally want to continue despite them. We do ignore the errors while mirroring the artifacts, however, if an error occured we return and do not mirror the metadata. As well, the ant task finishes by throwing an exception at the end. This is the cause of the IBuild failure http://download.eclipse.org/eclipse/downloads/drops/I20100519-0839/index.php
I think this should be fixed for 3.6
Created attachment 169205 [details] patch
The approach looks ok, but I don't know if this will fix our build failures (I don't know if there are other places that we need to check failOnError). A few observations 1. In some cases we mirrorStatus.getSeverity() == IStatus.ERROR, while in other places we result.matches(IStatus.ERROR). Is this inconsistency ok (this is not really related to the patch, just something I noticed)? 2. If we fail mirroring an artifact, but we continue to mirror the metadata, will we get into trouble later because our repository will be missing things? Are both of these problems related to the build failures? I wonder if the change to the MirroringApplication is necessary for this specific failure. (I'm not opposed to making the change, I'm just wondering if we need to risk it right now).
The build failure is being caused by the p2.mirror task with the ignoreErrors set when using the comparator. If there are errors, the metadata isn't mirrored. The errors aren't with the metadata itself. Rather, they are logged while using the comparator to compare the freshly built bundles with ones that already exist in the composite repository with the same id and version. If you use the old p2.artifact.mirror task, the ignoreErrors actually works, it just logs the errors to the file specified and the task doesn't fail the build. In the build, we discard new bundles with the same version and id and use the old ones from the composite repository. This is interesting to fix for 3.6 because 1) the p2.mirror task doesn't work as expected (ignoreErrors is ignored :-) 2) it would be nice to be able to exclude certain bundles from the comparator which can't be done using the old tasks (see bug 302283). I've reverted the build to use the old tasks in the interim.
(In reply to comment #3) > The approach looks ok, but I don't know if this will fix our build failures (I > don't know if there are other places that we need to check failOnError). > > A few observations > 1. In some cases we mirrorStatus.getSeverity() == IStatus.ERROR, while in other > places we result.matches(IStatus.ERROR). Is this inconsistency ok (this is not > really related to the patch, just something I noticed)? Status#matches is the correct way of doing this, technically, the severity is a bit field, so the proper check is actually (getSeverity & IStatus.ERROR != 0). In this particular case, the status we are looking at comes from Mirroring#run, and is a MultiStatus. Looking at MultiStatus#add, the severity is not used as a bit field, so == IStatus.ERROR works here. > > 2. If we fail mirroring an artifact, but we continue to mirror the metadata, > will we get into trouble later because our repository will be missing things? > > Are both of these problems related to the build failures? I wonder if the > change to the MirroringApplication is necessary for this specific failure. > (I'm not opposed to making the change, I'm just wondering if we need to risk it > right now). The problem here for builds is that when using a comparator, an Error does not imply that we were unable to mirror the artifact. It means we have 2 artifacts with the same version but different bytes. (When this happens, the original pre-existing artifact from the baseline is used). Ultimately, without this fix, you can't use the comparator on the p2.mirror task in the releng build because any differences will fail the build. The concern about transport errors and missing artifacts compared to metadata is valid, but is a larger enhancement that can't be made now. This problem is also present in the old p2.artifact.mirror tasks Kim is having to revert to. I raised bug 313733.
Thanks for the explanation everyone. +1 for the patch.
Thanks Ian.
