[GRADLE-1700] Tooling API M4 throws unsupported operation when getLinkedResources is called with an M3 distro Created: 28/Jul/11  Updated: 10/Feb/17  Resolved: 10/Feb/17

Status: Resolved
Project: Gradle
Affects Version/s: 1.0-milestone-3, 1.0-milestone-4
Fix Version/s: None

Type: Bug
Reporter: Kris De Volder Assignee: Unassigned
Resolution: Won't Fix Votes: 0


 Description   

The linked resources method throws an exception when it used on a project that uses Gradle 1.0 M3.

I thought the tooling API was meant to be backwards compatible?

To me backwards compatibility would mean that the methods added in M4 tooling API actually also work when used with a M3 distro.

If it is really not possible to implement this method in backwards compatible fashion then it should be made more clear that this method is not safe to call, and may throw an exception. So I'd prefer using checked rather than unchecked exceptions to indicate such problems.

Also the exception, is really hard to handle properly, as I really do not have any way to determine whether the exception indicates a real problem or not (I guess it all depends on whether or not the user's project requires linked resources).

It should be noted that, from my point of view, this is quite severe, since I will always call this method to configure imported projects. Throwing the exception means that the import will fail (even though it, most likely, is not a real problem in 90% of cases).

Thus, I'm tempted to ignore the exception, but don't think that's a good solution either, since it might sweep real problems under the rug, in some cases.



 Comments   
Comment by Adam Murdoch [ 28/Jul/11 ]

M3 does not provide any information about linked resources, and so the M4 tooling API has to do something when you ask for the linked resources for an M3 build. At the moment, it throws an exception to let you know the information is not available. We can tweak this behaviour to make it more useful. What would you prefer? Some options:

  • Throw UnsupportedVersionException instead of UnsupportedOperationException. We could possibly make UnsupportedVersionException a checked exception.
  • Return an empty list if the target Gradle version does not provide the linked resources.
  • Return null if the target Gradle version does not provide the linked resources.
  • Add another method boolean hasLinkedResources() which returns true only if the linked resources are available.
  • Add another method List<LinkedResource> getLinkedResources(List<LinkedResource> defaults) which return the linked resources, or the specified default if not available
  • Add another method List<LinkedResource> findLinkedResources() which returns the linked resources, or null/empty list if not available.
Comment by Adam Murdoch [ 28/Jul/11 ]

BTW, as a workaround for now, you can safely ignore the UnsupportedOperationException. The only time it will be thrown by the M4 implementation is when the value is not supported by the target Gradle version.

Comment by Kris De Volder [ 28/Jul/11 ]

Could the tooling API return an empty list for an M3 build, but only in the case where an empty list would actually be 'correct'?

I.e. only return an empty list in cases where the M4 API with an M4 distro on the same project would also return an empty list.

What I mean is, most projects don't have source folders like "../shared-src", which, if I understand correctly, is the main
reason why there are linked resources now. So for most projects returning empty is actually ok.

The problem I'm complaining about is that its impossible for me to distinguish between cases where its safe to ignore
the 'problem' and substitute an empty list and the (much rarer) cases where it is indicative of a real problem.

It seems to me that Gradle itself might be in a better position to make that determination. Maybe thats naive thinking and not
actually true.

The list of options above, are all very similar and not much better than what is implemented now.
i.e. all of them at best allow one to determine that the info is not available, but do not allow determining
whether this is a problem.

From that point of view, it is not extremely important which of them is actually implemented, but I'd have
a preference for a checked exception if those are the only choices.

Reason for my preferences is that a checked exception is the only one in the list that will really force the caller to think about the case
and write some code to handle it.

Anyway, I don't propose rushing in and making that change now. It may be better to think a bit more about how to
handle errors and problems in general first.

Comment by Kris De Volder [ 28/Jul/11 ]

In light of discussion, think its ok to lower priority from 'major' to 'minor' or trivial, (assuming that a substantial improvement in BW compatibility that I hoped for is practically not possible and minor changes in method signature are really not that important).

Comment by Szczepan Faber [ 14/Dec/11 ]

We had a discussion about the matter with Adam because we face similar problems internally. Here's the outcome:

  • As a short measure we will improve the exception type and the exception message. E.g. there's going to be a specific exception rather than jdk generic UnsupportedOperationException (for now we will extends from UOE, so there won't be any breaking changes). Also the message will describe the problem entirely and suggest that the user can safely handle and ignore this exception.
  • Mid-term feature we would like to provide different 'flavors' of models. At the moment, only the 'strict' model is supported, e.g. the model that throws exceptions eagerly when unsupported method is called. To shield the tooling api user from the complexity of handling those exceptions and knowing all the differences between gradle versions, we will introduce a 'lenient' model as well. So, one can use a lenient model of an EclipseProject and if she calls 'getLinkedResources()' method, it will return sensible default which in this particular case, would be simply an empty list. It makes sense if the tooling api decide on the defaults because the tooling api has the knowledge about the domain.
Comment by Kris De Volder [ 14/Dec/11 ]

I have a couple of suggestions:

1) methods which throw exceptions when they are 'missing' should make these exceptions checked exceptions.

I've now been 'bitten' twice already of expecting methods to work when they throw unexpected exceptions and thereby break the tooling. So I really think it will be nicer if the uncaught exceptions are flagged by the compiler.

2) any method throwing such exceptions should have clear Java docs specifying version requirements that makes them safe to use (i.e. as an API client I must be able to figure out easily under what conditions API will raise the exception, in terms of what version of Gradle the method was added in).

3) The API should provide a way to determine what version of gradle is being used. (So tools can make sensible decisions based on what functionality one can expect to work for a given project).

4) (I know this is hard... and may not be feasible in many cases but...) if at all possible the API should provide a 'backwards compatible' working implementation of newly added methods instead of throwing exceptions. If this is not the case, then it may be a very strong deterrent from migrating off off using older deprecated methods. Why? Because deprecated methods, which have been 'replaced' by newer methods will actually work more reliably across a wider range of versions than the newer methods.

Note: This point doesn't apply for getLinkedResources, because its not a case of replacing a deprecated method but is in fact a wholly new method, and the new functionality can't reasonably be expected to work on older versions of Gradle, so throwing exception is appropriate here (despite my initial complaints about it)

Comment by Kris De Volder [ 14/Dec/11 ]

PS: Personally I wouldn't mind a 'breaking change' if the reason for the change is to make 'missing method' exceptions that are presently being thrown in places that I may not even be aware of into 'checked' exceptions.

Sure it will 'break' my code, but in a good way (i.e. it will tell me where I have unwittingly done something that has a potential problem with uncaught exceptions).

Comment by Benjamin Muschko [ 15/Nov/16 ]

As announced on the Gradle blog we are planning to completely migrate issues from JIRA to GitHub.

We intend to prioritize issues that are actionable and impactful while working more closely with the community. Many of our JIRA issues are inactionable or irrelevant. We would like to request your help to ensure we can appropriately prioritize JIRA issues you’ve contributed to.

Please confirm that you still advocate for your JIRA issue before December 10th, 2016 by:

  • Checking that your issues contain requisite context, impact, behaviors, and examples as described in our published guidelines.
  • Leave a comment on the JIRA issue or open a new GitHub issue confirming that the above is complete.

We look forward to collaborating with you more closely on GitHub. Thank you for your contribution to Gradle!

Comment by Benjamin Muschko [ 10/Feb/17 ]

Thanks again for reporting this issue. We haven't heard back from you after our inquiry from November 15th. We are closing this issue now. Please create an issue on GitHub if you still feel passionate about getting it resolved.

Generated at Wed Jun 30 12:02:09 CDT 2021 using Jira 8.4.2#804003-sha1:d21414fc212e3af190e92c2d2ac41299b89402cf.