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.

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

allow apiDumpDirectory outside projectDir if inside rootProjectDir #239

Open
robstoll opened this issue Jun 13, 2024 · 14 comments
Open

allow apiDumpDirectory outside projectDir if inside rootProjectDir #239

robstoll opened this issue Jun 13, 2024 · 14 comments
Labels
enhancement New feature or request

Comments

@robstoll
Copy link

robstoll commented Jun 13, 2024

See #227 this would allow that one can specify a folder outside the project folder but still within the rootProject dir.

Say you have a multi-project setup with 10 sub-projects. Currently you can only define a folder within each sub-project but not a folder in the rootProject. This would be handy if you want to have all api files in one directory.

@fzhinkin fzhinkin added the enhancement New feature or request label Jun 21, 2024
@fzhinkin fzhinkin changed the title allow apiDumpDirectory outside projectDir if inside rootProjectDi allow apiDumpDirectory outside projectDir if inside rootProjectDir Jun 21, 2024
@robstoll
Copy link
Author

@fzhinkin I created an issue as you asked for in #227, how to proceed now? I did not get any feedback so far if there is a consensus that the current restriction is not needed if still inside the rootProjectDir.

@qwwdfsad
Copy link
Member

FTR: @fzhinkin is taking a break, so please expect some delayed response times

@robstoll
Copy link
Author

@qwwdfsad maybe you can give me pointers how to proceed? If the request as such is accepted, then I gladly add tests if needed.

@fzhinkin
Copy link
Collaborator

I agree that storing all dumps in a single directory somewhere under the rootProject's dir could be handy.

The main issue I see here from the design point of view is how all these ABI files should be stored within a single directory. For projects using a flat modules hierarchy everything is simple:

library
├── api
│   ├── core.api
│   └── extras.api
├── core
└── extras

In this case, we can continue using old file names and put everything into a single directory (library/api, or whichever name a user will choose).

However, if a project has a deeper submodule hierarchy, things are getting complicated:

library
├── api
│   ├── ????
├── core
│   ├── main
│   └── extras
└── extras

Here, it's no longer clear how generated API files should be stored within the library/api dir. Should the api directory use the same directories hierarchy as modules (i.e., library/api/extras.api, library/api/core/main.api, library/api/core/extras.api) or should it use flat files list? In the latter case, we need to distinguish library:core:extras from library:extras, so some alternative file naming scheme is required.

@robstoll
Copy link
Author

robstoll commented Aug 13, 2024

I suggest we don't change the current behaviour and still dump in the subdirectory per default but allow that one can define a directory within the rootProject dir (see the mentioned PR). It is then up to the project maintainers how they store the files and what naming scheme they define.

If you prefer to change the default behaviour, then I suggest we replace : with - in the name and generate all files in one directory per default. One can still change it if they prefer another schema etc.

@fzhinkin
Copy link
Collaborator

I suggest we don't change the current behaviour and still dump in the subdirectory per default but allow that one can define a directory within the rootProject dir (see the mentioned PR). It is then up to the project maintainers how they store the files and what naming scheme they define.

Once one had configured the plugin to use a single directory within a rootProject to store dumps generated for all subprojects there, it's up to BCV how to put all the files into that directory avoiding naming conflicts along the way. Maintainers cannot control the naming scheme as BCV does not provide an extension point for that.

@robstoll
Copy link
Author

robstoll commented Aug 13, 2024

ah true, my bad, they cannot modify the naming schema but modifying the directory structure would be enough to workaround name clashes.

Of course, if we want that one is able to put everything in one directory in all cases and still be able to workaround a name clash then I guess we should provide a way to modify the file name as well and use project.path as default but skip the first : and replace the remaining : with -. i.e. library:core:extras becomes core-extras and library:extras becomes extras and in case there is a library:core-extras project then we will fail for library:core:extras and point to the property which allows to manage the file name. WDYT?

@fzhinkin
Copy link
Collaborator

and in case there is a library:core-extras project then we will fail for library:core:extras and point to the property which allows to manage the file name. WDYT?

Currently, BCV could only be configured for the top level project, so there's no way to set some properties within subprojects.

We can, of course, support a top-level property allowing to configure name mapping for each subproject, but I would rather have a robust naming scheme out of the box.

@robstoll
Copy link
Author

I see, so how about we use _ as replacement for : instead of -. I think - is more often used in project names than _, so with _ we will run already into less ambiguities and then we could escape _ in a project name with e.g. double __. This is still not perfect as we can see in the following:

  • library:core:extras => core_extras.api
  • library:core_extras => core__extras.api
  • library:core__extras => core____extras.api
  • library:core:_extras => core___extras.api
  • library:core_:extras => core___extras.api (! same name)

So we need to define another escaping for either prefix or suffix _ in project names. I suggest we escape the suffix as I doubt that there will be a lot of cases where project names have a _ suffix. I suggest the following escaping:

  • library:core:_extras => core___extras.api
  • library:core_:extras => core-_-_extras.api
  • library:core-_-:extras => core-__-_extras.api
  • library:core:-_-extras => core_-__-extras.api

WDTY?

@fzhinkin
Copy link
Collaborator

@robstoll, maybe we can simply use : as a delimiter in dump files names? Gradle's project names could not contain :, so there would be no clashes (and, well, we can just use a project path as a name's prefix).

@robstoll
Copy link
Author

Personally, I wouldn't mind but Windows users would be blocked I guess because the file system doesn't allow : in file names

@fzhinkin
Copy link
Collaborator

Perhaps, we should make a separator configurable and use "_" a default one. Then, we can check for name clashes during the plugin configuration, print colliding name and force user to pick an alternative separator.

This way, we'll keep things simple, and there will always be the workaround for projects with bizarre subproject names.

@robstoll
Copy link
Author

good with me, I doubt that users will have a lot of conflicts if we choose _ as default. Shall I extend my (closed) PR with this functionality or are you going to create one?

@fzhinkin
Copy link
Collaborator

Shall I extend my (closed) PR with this functionality or are you going to create one?

Yes, please, go ahead. Currently, I don't have a time budget for this particular issue, so your contributions are welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants