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

Darker did rely on --config but now only uses black find_project_root for it's own config, breaking mono-repository usage #244

Closed
DavidCDreher opened this issue Dec 7, 2021 · 7 comments · Fixed by #359
Assignees
Labels
bug Something isn't working move later? question Further information is requested
Milestone

Comments

@DavidCDreher
Copy link
Collaborator

We have a mono-repo layout something like this

Monorepo/
├── pyproject.toml
├── lib/
│   ├── some_lib/
│   │   ├── setup.py
│   │   └── pyproject.toml
│   └── another_lib/
│       ├── setup.py
│       └── pyproject.toml
└── apps/
    └── some_app/
        ├── setup.py
        └── pyproject.toml

The root pyproject.toml contains the [tool.darker] and [tool.black] configuration for the whole project.
The individual apps/libs pyproject.toml files contain build and other settings.

You used to have the load_config for darker respect the --config argument:

def load_config(path: Optional[str], srcs: List[str]) -> DarkerConfig:

But unfortunately you removed this:

def load_config(srcs: Iterable[str]) -> DarkerConfig:

Now if we have changed for example lib/some_lib/setup.py then the sources that get passed are src = [ "lib/some_lib/setup.py" ] which get passed to black's find_project_root which will find the pyproject.toml file in lib/some_lib/pyproject.toml and you will load that, which has no settings for darker. It would be good to revert this or at least allow the option to configure the path to the darker config file as well.

@akaihola
Copy link
Owner

Hi @DavidCDreher,

Thanks for notifying about this unfortunate regression, and for a detailed description.

I'll get back to this very soon. Did you make any attempt to modify Darker to restore the old behavior?

@DavidCDreher
Copy link
Collaborator Author

DavidCDreher commented Dec 13, 2021

Hi @akaihola since we mainly use darker from pre-commit we simply modified our pre-commit configuration to contain all the options as arguments. That's an OK fix for us, so there's no time pressure here. I was just able to provide a detailed description because it was driving me mad. The current behavior means it picks up different configuration files, depending on which files were modified in a diff/PR and we had the issue that behavior was different locally vs in the CI pipeline so I went digging. But this means I can't provide you with a patch :/

@akaihola akaihola added the bug Something isn't working label Jan 18, 2022
@akaihola akaihola self-assigned this Jan 18, 2022
@akaihola akaihola added this to the 1.4.0 milestone Jan 18, 2022
@akaihola akaihola modified the milestones: 1.4.0, 1.4.1, 1.4.2 Feb 8, 2022
@akaihola
Copy link
Owner

Hi @DavidCDreher,

Would it be a better behavior if Darker ignored pyproject.toml files which don't contain a [tool.darker] section, and would continue towards the filesystem root until it either finds a .git subdirectory or a [tool.darker] in pyproject.toml?

I do regret that the --config option specifies the Black and/or isort configuration file, instead of pointing to Darker's own configuration. I wonder if we could do a long deprecation period, introduced --black-config and --isort-config and change --config to explicitly be about Darker configuration only?

@akaihola akaihola added move later? question Further information is requested labels Feb 27, 2022
@akaihola
Copy link
Owner

I'm moving this to release 1.5.0 since 1.4.2 is coming out any day now.

@akaihola
Copy link
Owner

akaihola commented Apr 1, 2022

introduce --black-config and --isort-config and change --config to explicitly be about Darker configuration only

#351 would actually make it possible to simply do --black-options="--config <path>" instead of --black-config=<path>.

@DavidCDreher
Copy link
Collaborator Author

Sorry I just realized there was a question along your comments:

Would it be a better behavior if Darker ignored pyproject.toml files which don't contain a [tool.darker] section, and would continue towards the filesystem root until it either finds a .git subdirectory or a [tool.darker] in pyproject.toml?

I think first and foremost there should be way to supply the directory where to find the config and that should be always respected by the tool. I can understand blacks approach here on trying to traverse the tree until it finds a pyproject.toml file since it makes command line usage pretty straight forward. But I would say your approach would be better. That way it would respect any higher level configuration and in case of lower level ones you would need to explicitly overwrite the settings, which is nicely explicit.

@akaihola
Copy link
Owner

akaihola commented Apr 4, 2022

Thanks @DavidCDreher for your reply!

Combining my and your comments, I think we haven't yet precisely defined how Darker should work (eventually, and possibly in interim versions during deprecation).

Would the following be a fair approach in your opinion?

Version 1.5.0

  • read Darker configuration from the file specified using -c/--config again (like it used to be)
  • assume <directory>/pyproject.toml if the value of -c/--config is a directory
  • (Update: these changes are now in Use -c PATH/--config PATH for Darker config #359)

Version 1.6.0

  • introduce --black-config=<path> to just specify where Black's configuration is, but still use --config for it too (with a lower precendence)
  • introduce --isort-config=<path> to just specify where isort's configuration is, but still use --config for it too (with a lower precendence)
  • deprecate reading isort and Black configuration from the file specified using -c/--config:
    • show a deprecation warning if [tool.isort] or [tool.black] exists in the file defined using --config and if the corresponding --isort-config/--black-config option is omitted

Version 2.0.0

  • no longer read Black and isort configuration from the file specified using -c/--config
  • in absence of -c/--config, or --black-config, or --isort-config, traverse the directory tree towards the root to find a pyproject.toml, but ignore files which don't contain the corresponding [tool.*] section

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working move later? question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants