Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 318818 - IXtextBuilderParticipant: deltas contain too much information
Summary: IXtextBuilderParticipant: deltas contain too much information
Status: CLOSED WONTFIX
Alias: None
Product: TMF
Classification: Modeling
Component: Xtext (show other bugs)
Version: 1.0.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-04 13:21 EDT by Victor Noël CLA
Modified: 2017-09-19 16:35 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Noël CLA 2010-07-04 13:21:10 EDT
Hello,

I noticed some things that I would qualify as bugs, but maybe I am mistaken.

This all happen in the build method of IXtextBuilderParticipant.

1) If I change in a dsl file an element that is referenced in another file, then the referencing file will also be present in a delta.

2) The dsl files are also present in the output directory, and there is some deltas about these files. I don't think they should be taken into account.

Thank you
Comment 1 Sebastian Zarnekow CLA 2010-07-05 01:44:51 EDT
Hi Victor,

can you confirm that delta.hasChanges() returns false for the deltas that you considered superfuous?

What do you mean by output directory? /src-gen or /bin?

Regards,
Sebastian
Comment 2 Victor Noël CLA 2010-07-05 05:36:05 EDT
(In reply to comment #1)
> can you confirm that delta.hasChanges() returns false for the deltas that you
> considered superfuous?

It is as you said, delta.haveEObjectDescriptionsChanged() returns false for the deltas concerning other files than the one changed.
So this does not seems like a bug, even if I am wondering what is the point of having these deltas, just to know about what was impacted by the change (that's a good point :)

But it stills return true for the deltas of the changed file in the output directory.

> 
> What do you mean by output directory? /src-gen or /bin?

The /bin one, but it does not happen with every project I have.
I have a test project for developing my DSL, and files in the /bin directory are not parsed…
But I have also a bigger project (using maven, so the output directory is /target/classes, I don't know if it is important) and for this one, I get the problematic behaviour.

Things happens as follow, there is 3 files, common, logic and robot. logic and robot are referencing elements declared in common. common is modified (with something that has no impact on the other files) and saved.

1) IXtextBuilderParticipant's build is called, there is 5 deltas (the boolean is the haveEObjectDescriptionsChanged()):
platform:/resource/rosace-agent/target/classes/robot.muadlt: false
platform:/resource/rosace-agent/src/main/resources/common.muadlt: true
platform:/resource/rosace-agent/src/main/resources/logic.muadlt: false
platform:/resource/rosace-agent/target/classes/logic.muadlt: false
platform:/resource/rosace-agent/src/main/resources/robot.muadlt: false


2) build is called again, I guess because some code was generated in src-gen so it triggers it or something like that, there is 3 deltas:
platform:/resource/rosace-agent/target/classes/logic.muadlt: false
platform:/resource/rosace-agent/target/classes/common.muadlt: true
platform:/resource/rosace-agent/target/classes/robot.muadlt: false

3) build is called, again, for the same reasons I guess (since the previous one generated some code again), and there is no deltas.

I would think that platform:/resource/rosace-agent/target/classes/ should not be read.

Thank you
Comment 3 Victor Noël CLA 2010-07-05 10:07:03 EDT
(In reply to comment #2)
> (In reply to comment #1)
> > can you confirm that delta.hasChanges() returns false for the deltas that you
> > considered superfuous?
> 
> It is as you said, delta.haveEObjectDescriptionsChanged() returns false for the
> deltas concerning other files than the one changed.
> So this does not seems like a bug, even if I am wondering what is the point of
> having these deltas, just to know about what was impacted by the change (that's
> a good point :)

I would like to add that it is difficult to rely on delta.haveEObjectDescriptionsChanged() because it does not seem to take into account elements that were not exported (I have the feeling it is based on the name property, but I can't be sure).
For example if I modify the content of an element but not the element itself, then the delta is not marked as changed.
Comment 4 Sven Efftinge CLA 2010-07-05 10:31:59 EDT
(In reply to comment #3)
> I would like to add that it is difficult to rely on
> delta.haveEObjectDescriptionsChanged() because it does not seem to take into
> account elements that were not exported (I have the feeling it is based on the
> name property, but I can't be sure).

That's the intended behavior. We only need to inform dependency if something they reference has changed. The default is name changes, but it can and should be adapted in the org.eclipse.xtext.resource.IResourceDescription.Manager.isAffected(Delta, IResourceDescription) if that is not enough.
Comment 5 Victor Noël CLA 2010-07-05 11:04:29 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > I would like to add that it is difficult to rely on
> > delta.haveEObjectDescriptionsChanged() because it does not seem to take into
> > account elements that were not exported (I have the feeling it is based on the
> > name property, but I can't be sure).
> 
> That's the intended behavior. We only need to inform dependency if something
> they reference has changed. The default is name changes, but it can and should
> be adapted in the
> org.eclipse.xtext.resource.IResourceDescription.Manager.isAffected(Delta,
> IResourceDescription) if that is not enough.

Ok, but I am not sure this is what I am talking about, the problem here is not about an element in a resource referencing an element present in the delta, but about an element in the delta containing an element that does not have a name but that changed anyway and because of that the delta should return true for haveEObjectDescriptionsChanged().
So it's more about how haveEObjectDescriptionsChanged() is implemented (or about how to compare two elements of the delta) for a delta than about the implication of a delta on another resource.

Of course what I say would fall apart if deltas were also internally used to represent changes in elements inside another element, but I didn't had the feeling it was this way…
Comment 6 Sebastian Zarnekow CLA 2010-07-30 12:56:13 EDT
(In reply to comment #5)
> Ok, but I am not sure this is what I am talking about, the problem here is not
> about an element in a resource referencing an element present in the delta, but
> about an element in the delta containing an element that does not have a name
> but that changed anyway and because of that the delta should return true for
> haveEObjectDescriptionsChanged().

Please use the user data field to store additional information about the relevant state of the object, e.g. a hashCode for all the contents that should be considered in the haveDescriptionsChanged(

> Of course what I say would fall apart if deltas were also internally used to
> represent changes in elements inside another element, but I didn't had the
> feeling it was this way…

Deltas are used to compute the affected resources in the builder / among open editors and to provide as much information for listeneres / builder participants as possible.
Comment 7 Sebastian Zarnekow CLA 2010-08-19 08:04:26 EDT
I'ld like to close this one. Any objections?
Comment 8 Sebastian Zarnekow CLA 2010-08-24 08:52:49 EDT
Closed as won't fix.
Comment 9 Karsten Thoms CLA 2017-09-19 16:35:38 EDT
Closing bug which were set to RESOLVED before Eclipse Neon.0.