Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for repository tags #6116

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Add support for repository tags #6116

wants to merge 13 commits into from

Conversation

chrisrueger
Copy link
Contributor

@chrisrueger chrisrueger commented May 8, 2024 •
edited

Closes #5966

Based on brainstorming here https://bnd.discourse.group/t/baseline-repo-exclude-from-the-build-path/391/6

In this PR

  • allow to add a property tags="baseline,release, foo" to a Repository in build.bnd
  • default tag in all repositories without a tag is an empty set of tags for backwards compatibility
  • in a .bndrun if you remove the -runrepos you get all repositories which have either no tags set or the 'resolve' tag
  • To exclude a repo from resolution in the example, you would not add the resolve (but instead any other tag e.g. <<EMPTY>> and remove the -runrepos from the .bndrun.
  • If you have specified -runrepos then today's behavior is still in place that the given repo names are used

e.g. a .bndrun file without -runrepos would include the following repos in the resolution:

aQute.bnd.repository.maven.provider.MavenBndRepository;\
        tags = "resolve"; \
        name="Maven Central A";\
        releaseUrl="${mavencentral}";\
        snapshotUrl="${ossrh}";\
        index="${.}/central.mvn";\
        readOnly=true,\

because it has the resolve tag

and

aQute.bnd.repository.maven.provider.MavenBndRepository;\
        name="Maven Central B";\
        releaseUrl="${mavencentral}";\
        snapshotUrl="${ossrh}";\
        index="${.}/central.mvn";\
        readOnly=true,\

because there is no tags

but not

aQute.bnd.repository.maven.provider.MavenBndRepository;\
        tags = "someothertag"; \
        name="Maven Central";\
        releaseUrl="${mavencentral}";\
        snapshotUrl="${ossrh}";\
        index="${.}/central.mvn";\
        readOnly=true,\

because it has a no resolve tag.

To exclude the baseline repo in the example, you would not add the resolve tag to the baseline-repo, and then it won't be considered for Resolving. I have not used baselining yet, so I am not familiar with the topic and what areas are not covered by this.

The current state of the PR just lay the foundation for tagging. More features could be added like allowing for NOT or more sophisticated filter expressions.

@chrisrueger
allow tagging of repositories
52af186 
e.g. add a property tags="baseline,release, foo";\ to a Repository in build.bnd

tags="baseline,release, foo";\

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
use Set
ea7740d 
Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
add tagged support to more repos and edit dialog
359a405 
Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
allow to use tag in -runrepos in .bndrun
dd36fe1 
example:
-runrepos: \
	Workspace,\
	@resolve

this adds the Workspace repo and all other repos tagged with the tag 'resolve'

I used the '@' character as a marker for tags (for now). initially I wanted to use '#' but this is a comment in a .bndrun file.

With this i got a successful resolution as before

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
Copy link
Contributor Author

Summary from the bnd zoom call today:

  • revert the Respositories Panel support for tags (e.g. @resolve) in -runrepos... just keep it a list of workspace names and a way to be specific
  • start a step back with an assumed empty -runrepos list or no -runrepos at
    • in this case the repostories should be fetched via something like repos = getPlugins(..., "resolve") - the second param should be the tag resolve which should be one a handful of fixed core-tags
    • every Repository should automatically have this tag resolve tag

That's it so far. I will post an update once I have this implemented and from there we can discuss the next step.

Thanks @pkriens for the input.

@chrisrueger
rework - repos now have 'resolve' tag by default
b9a574f 
- this commit partly reverts previous commit
- repos now get the 'resolve' tag by default if not specified
- .bndrun: empty -runrepos will be populated with all repos having the 'resolve' tag
- .bndrun: resolution consider all repos having the 'resolve' tag. that means you can exclude a repo from resolution by manually assigning it a different tag (e.g. to exclude the baseline repo from resolution, you should give the baseline-repo e.g. the tag 'baseline' and make sure it does NOT have the 'resolve' tag

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
Copy link
Contributor Author

  • revert the Respositories Panel support for tags (e.g. @resolve) in -runrepos... just keep it a list of workspace names and a way to be specific

  • start a step back with an assumed empty -runrepos list or no -runrepos at

    • in this case the repostories should be fetched via something like repos = getPlugins(..., "resolve") - the second param should be the tag resolve which should be one a handful of fixed core-tags
    • every Repository should automatically have this tag resolve tag

@pkriens I have implemented the above.

  • if -runrepos is empty (or not in the .bndrun file) the UI shows all repos having the 'resolve' tag
    • if you don't touch any checkbox, then it stays that way
    • if you touch a checkbox then the repo-names are written to .bndrun and basically override the 'resolve' tag behavior.
    • I guess to make use of the new tagging the recommendation would be to remove -runrepos from .bndrun to make use of the tags

Example: this is my Repositories list with no -runrepos specified

image

It is basically all repos with the resolve tag (repos have not tags specified, so the default 'resolve' is used)

Resolution:

  • basically uses a similar behavior
  • if -runrepos is empty (or not in the .bndrun file) resolution uses all repos having the 'resolve' tag. Otherwise the -runrepos are used

Quoting commit b9a574f

  • repos now get the 'resolve' tag by default if not specified
  • .bndrun: empty -runrepos will be populated with all repos having the 'resolve' tag
  • .bndrun: resolution consider all repos having the 'resolve' tag. that means you can exclude a repo from resolution by manually assigning it a different tag (e.g. to exclude the baseline repo from resolution, you should give the baseline-repo e.g. the tag 'baseline' and make sure it does NOT have the 'resolve' tag

Let's discuss

@chrisrueger chrisrueger marked this pull request as ready for review May 15, 2024 09:00
pkriens
pkriens reviewed May 17, 2024
View reviewed changes
Copy link
Member

@pkriens pkriens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. You can take the recommendations:

  • extend RepositoryPlugin with Tagged
  • Maybe make a Tags class, extending Set
  • not parsing the string all the time

biz.aQute.bndlib/src/aQute/bnd/service/Tagged.java Outdated Show resolved Hide resolved
biz.aQute.bndlib/src/aQute/bnd/service/Tagged.java Outdated Show resolved Hide resolved
biz.aQute.bndlib/src/aQute/lib/deployer/FileRepo.java Outdated Show resolved Hide resolved
biz.aQute.repository/test/aQute/bnd/repository/maven/provider/WorkspaceTest.java Outdated Show resolved Hide resolved
biz.aQute.bndlib/src/aQute/bnd/osgi/repository/BaseRepository.java Show resolved Hide resolved
@chrisrueger
Merge remote-tracking branch 'upstream/master' into add-repository-tags
34091a8
@chrisrueger
implement suggestions
c08cce3 
- extend RepositoryPlugin with Tagged
- make a Tags class, extending Set
- not parsing the string all the time
- replace RepoTags enum with Constants.REPOTAGS_RESOLVE

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
move matchesTags back to Tagged
e990f07 
Somehow I think it belongs there.

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
Tags implement Set using an internal sorted set
7857d51 
fix and improve testcase to test for consistent sorting of tags
Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
improve tag matching
53cb0dc 
- keep backwards compatibility so that Repos without tags return an empty set and empty set means "matches"
- renamed Tags.matchesTags to Tags.isIncluded
- move tags to own package aQute.bnd.service.tags

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
mention <<EMPTY>> placeholderin repo plugin UI
e46fd43 
Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
make tags a combobox in Plugins UI
7f04909 
Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
@chrisrueger
Copy link
Contributor Author

@pkriens I have pushed the changes from our call today.
In the UI I have implemented a simple Combobox mechanism where you can provide the options using a comma-separated string.

see 7f04909

image

@chrisrueger
rename to Tags.includesAny
7313fbc 
I think that better reflects the intent.
- also adjusting javadoc accordingly
- and move static methods to bottom

Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkriens pkriens pkriens left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add runrepos blacklist
2 participants
@chrisrueger @pkriens