You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by GitBox <gi...@apache.org> on 2021/12/30 10:31:33 UTC
[GitHub] [felix-dev] HannesWell opened a new pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
HannesWell opened a new pull request #124:
URL: https://github.com/apache/felix-dev/pull/124
This PR addresses issue [FELIX-6493](https://issues.apache.org/jira/browse/FELIX-6493) and extends the is-up-to-date logic of the Manifest goal to
- consider as not up-to-date if the MANIFEST.MF file to generate does not exist
- consider pom.xml modifications in up-to-date logic
- write incremental-info after manifest to not self-cause out-of-date
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003575734
> > > > But except this case, I don't know how to disable the write of compiled class files into target/classes directory?
> > >
> > >
> > > I mean that writing into a directory also updates the directory timestamp!
> >
> >
> > Ah, sorry I didn't understand that. That's not a problem because the `newer()` methods walks the entire file-tree and checks each (transitively) contained file if a checked path is a directory. For windows I don't even know if it is possible to update the timestamp of the containing directory if a contained element changes. At least by default this does not happen.
>
> Okay it seems I misread your comment then, I though you have proposed to only check the timestmap of the folder instead of its contents.
No no. Sorry for being unclear. My proposal is to skip the `buildContext.newScanner(directory)` in anyJavaSourceFileTouchedSinceLastBuild() (and maybe rename the method accordingly), which scans for new java files since the last build (if I understood the doc correctly).
But this should go into a separate clean-up change as suggested in my first comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1012447167
@jbonofre, @cziegeler, hboutemy or @gnodet could you be so kind and review this change or ping somebody who can?
We would really appreciate if this change would be part of the next release and won't miss the next one too. Please let me know if I can do anything to speed up the review process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003567910
> > But except this case, I don't know how to disable the write of compiled class files into target/classes directory?
>
> I mean that writing into a directory also updates the directory timestamp!
Ah, sorry I didn't understand that.
That's not a problem because the `newer()` methods walks the entire file-tree and checks each (transitively) contained file if a checked path is a directory.
For windows I don't even know if it is possible to update the timestamp of the containing directory if a contained element changes. At least by default this does not happen.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] laeubi commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003047363
> From my recent observations the BND-Analyzer takes some time for larger projects with many classes to scan. So I think it is not worthless.
Sure but how often is an incremental build triggered that does *not* is detected as a change? e.g if I see correctly it is only checked if a java file is affected but not if the change actually is a relevant change (e.g. if I just add a comment into a file this will still trigger generation of the manifest!).
> So additional or modified java classes while cause later modifications in that directory
I'm not sure if this is always true as far as I know under linux one could disable that parents are updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003553742
> > From my recent observations the BND-Analyzer takes some time for larger projects with many classes to scan. So I think it is not worthless.
>
> Sure but how often is an incremental build triggered that does _not_ is detected as a change? e.g if I see correctly it is only checked if a java file is affected but not if the change actually is a relevant change (e.g. if I just add a comment into a file this will still trigger generation of the manifest!).
In general a change is not detected, if a non java resource is modified that is not copied into `target/classes`. It then depends on ones use cases/workspace. For some cases (like e.g. in our case in M2E where the project actually only contains a pom that specifies what is packed into a bundle) there are probably only very few cases where the manifest generation is skipped. But others might have other cases.
>
> > So additional or modified java classes while cause later modifications in that directory
>
> I'm not sure if this is always true as far as I know under linux one could disable that parents are updated.
Of course one could disable autobuild in Eclipse (I don't know how other IDE handle it), but then no incremental builds happen anyways. But except this case, I don't know how to disable the write of compiled class files into target/classes directory?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] laeubi commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003033763
> > * consider pom.xml modifications in up-to-date logic
>
> Actually the pom's of all parent projects have to be considered as well. I'm about to update this PR accordingly.
I'm not sure if this is not a bit out of scope. I would assume that if one updates the parent he also has to update the child to reflect the new version?
What I actually wonder is: Is there really a measurable benefit for all those stale checking? I could only think of a resource that was modified and thus do not change the generated manifest but does it really impact performance to regenerate the manifest in those scenarios?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] laeubi commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003566629
> But except this case, I don't know how to disable the write of compiled class files into target/classes directory?
I mean that writing into a directory also updates the directory timestamp!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] laeubi commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003574787
@tjwatson @gnodet can you review/merge this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell edited a comment on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell edited a comment on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003042843
> > > * consider pom.xml modifications in up-to-date logic
> >
> >
> > Actually the pom's of all parent projects have to be considered as well. I'm about to update this PR accordingly.
>
> I'm not sure if this is not a bit out of scope. I would assume that if one updates the parent he also has to update the child to reflect the new version?
For incremental builds in IDE's like Eclipse this is actually useful. If one modifies the parent pom (in a way that also affects the child's generated Manifest), Eclipse triggers an incremental build of parent and child project (I assume the ResourcesPlugin checks the project inter-dependencies). If the manifest plugin does not check the parent pom it falsely assume the manifest is up to date (because the child's pom did not change) and skips the execution. In order to re-generated the manifest a user would have to trigger a full build or modify the child pom manually. But especially during development between releases it is not necessary to change to version of the parent which would require adjustments of the child,
> What I actually wonder is: Is there really a measurable benefit for all those stale checking? I could only think of a resource that was modified and thus do not change the generated manifest but does it really impact performance to regenerate the manifest in those scenarios?
From my recent observations the BND-Analyzer takes some time for larger projects with many classes to scan. So I think it is not worthless. At the moment `biz.aQute.bndlib` version 5.1.1 is used while 6.1.0 is available maybe the new major version also brings performance improvements.
But what I'm pretty confident about is that checking for new or modified Java-source files in anyJavaSourceFileTouchedSinceLastBuild() is not necessary because `isUpToDate()` checks the ${project.build.outputDirectory} for later modifications and the compiled class files are written there. So additional or modified java classes will cause later modifications in that directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003042843
> > > * consider pom.xml modifications in up-to-date logic
> >
> >
> > Actually the pom's of all parent projects have to be considered as well. I'm about to update this PR accordingly.
>
> I'm not sure if this is not a bit out of scope. I would assume that if one updates the parent he also has to update the child to reflect the new version?
For incremental builds in IDE's like Eclipse this is actually useful. If one modifies the parent pom (in a way that also affects the child's generated Manifest), Eclipse triggers an incremental build of parent and child project (I assume the ResourcesPlugin checks the project inter-dependencies). If the manifest plugin does not check the parent pom it falsely assume the manifest is up to date (because the child's pom did not change) and skips the execution. In order to re-generated the manifest a user would have to trigger a full build or modify the child pom manually. But especially during development between releases it is not necessary to change to version of the parent which would require adjustments of the child,
> What I actually wonder is: Is there really a measurable benefit for all those stale checking? I could only think of a resource that was modified and thus do not change the generated manifest but does it really impact performance to regenerate the manifest in those scenarios?
From my recent observations the BND-Analyzer takes some time for larger projects with many classes to scan. So I think it is not worthless. At the moment `biz.aQute.bndlib` version 5.1.1 is used while 6.1.0 is available maybe the new major version also brings performance improvements.
But what I'm pretty confident about is that checking for new or modified Java-source files in anyJavaSourceFileTouchedSinceLastBuild() is not necessary because `isUpToDate()` checks the ${project.build.outputDirectory} for later modifications and the compiled class files are written there. So additional or modified java classes while cause later modifications in that directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1002968329
Besides the changes affecting the logic a moderate number of possible clean up changes.
To you want to have a dedicated issue for a clean up or can I create a subsequent PR without associated issue or should these changes go into this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] HannesWell commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
HannesWell commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1002992144
> - consider pom.xml modifications in up-to-date logic
Actually the pom's of all parent projects have to be considered as well.
I'm about to update this PR accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [felix-dev] laeubi commented on pull request #124: [FELIX-6493] Extend is-up-to-date logic of Manifest goal
Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #124:
URL: https://github.com/apache/felix-dev/pull/124#issuecomment-1003568832
> > > But except this case, I don't know how to disable the write of compiled class files into target/classes directory?
> >
> >
> > I mean that writing into a directory also updates the directory timestamp!
>
> Ah, sorry I didn't understand that. That's not a problem because the `newer()` methods walks the entire file-tree and checks each (transitively) contained file if a checked path is a directory. For windows I don't even know if it is possible to update the timestamp of the containing directory if a contained element changes. At least by default this does not happen.
Okay it seems I misread your comment then, I though you have proposed to only check the timestmap of the folder instead of its contents.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org