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

Use --config in ecosystem checks #10345

Open
zanieb opened this issue Mar 11, 2024 · 4 comments · May be fixed by #10436
Open

Use --config in ecosystem checks #10345

zanieb opened this issue Mar 11, 2024 · 4 comments · May be fixed by #10436
Labels
ci Related to internal CI tooling help wanted Contributions especially welcome

Comments

@zanieb
Copy link
Member

zanieb commented Mar 11, 2024

Our ecosystem checks manually overwrite the pyproject.toml file to perform some overrides but now that we have --config support we should use that instead.
@zanieb zanieb added help wanted Contributions especially welcome ci Related to internal CI tooling labels Mar 11, 2024
@hoel-bagard
Copy link
Contributor

I haven't touched at this part of the ruff codebase yet, but I would be interested in trying to fix this issue.

Would you have links to help me get started ?

@zanieb
Copy link
Member Author

zanieb commented Mar 13, 2024 •
edited

Yeah basically I think we can replace this whole blurb

ruff/python/ruff-ecosystem/ruff_ecosystem/projects.py

Lines 89 to 165 in c269c1a

@contextlib.contextmanager
def patch_config(
self,
dirpath: Path,
preview: bool,
) -> None:
"""
Temporarily patch the Ruff configuration file in the given directory.
"""
dot_ruff_toml = dirpath / ".ruff.toml"
ruff_toml = dirpath / "ruff.toml"
pyproject_toml = dirpath / "pyproject.toml"
# Prefer `ruff.toml` over `pyproject.toml`
if dot_ruff_toml.exists():
path = dot_ruff_toml
base = []
elif ruff_toml.exists():
path = ruff_toml
base = []
else:
path = pyproject_toml
base = ["tool", "ruff"]
overrides = {
**ALWAYS_CONFIG_OVERRIDES,
**self.always,
**(self.when_preview if preview else self.when_no_preview),
}
if not overrides:
yield
return
# Read the existing content if the file is present
if path.exists():
contents = path.read_text()
toml = tomli.loads(contents)
else:
contents = None
toml = {}
# Do not write a toml file if it does not exist and we're just nulling values
if all((value is None for value in overrides.values())):
yield
return
# Update the TOML, using `.` to descend into nested keys
for key, value in overrides.items():
if value is not None:
logger.debug(f"Setting {key}={value!r} in {path}")
else:
logger.debug(f"Restoring {key} to default in {path}")
target = toml
names = base + key.split(".")
for name in names[:-1]:
if name not in target:
target[name] = {}
target = target[name]
if value is None:
# Remove null values i.e. restore to default
target.pop(names[-1], None)
else:
target[names[-1]] = value
tomli_w.dump(toml, path.open("wb"))
try:
yield
finally:
# Restore the contents or delete the file
if contents is None:
path.unlink()
else:
path.write_text(contents)

with appending a --config option at

ruff/python/ruff-ecosystem/ruff_ecosystem/projects.py

Lines 204 to 225 in c269c1a

def to_ruff_args(self) -> list[str]:
args = [
"check",
"--no-cache",
"--exit-zero",
# Ignore internal test rules
"--ignore",
"RUF9",
# Use the concise format for comparing violations
"--output-format",
"concise",
f"--{'' if self.preview else 'no-'}preview",
]
if self.select:
args.extend(["--select", self.select])
if self.ignore:
args.extend(["--ignore", self.ignore])
if self.exclude:
args.extend(["--exclude", self.exclude])
if self.show_fixes:
args.extend(["--show-fixes", "--ecosystem-ci"])
return args

The idea is to provide the options directly instead of writing them to a file

@hoel-bagard
Copy link
Contributor

@zanieb I started looking into this, and I have a few things I would like to ask:

1. How to unset / restore to default using --config

One of the overrides is required-version = null. Currently this just removes the entry from the toml file, however I don't know how to do that from the CLI with --config (since toml does not have null).

2. Where to make the changes

In order to keep CheckOptions and ConfigOverrides separated, I'm thinking of adding the overrides to the list returned by CheckOptions.to_ruff_args():

    ruff_args = options.to_ruff_args()
    ruff_args.append(overrides)

But I could also do it inside of the to_ruff_args method by changing its signature to the one below.

    def to_ruff_args(
        self,
        config_overrides: ConfigOverrides | None = None,
    ) -> list[str]:

What do you think about it ?

@zanieb
Copy link
Member Author

zanieb commented Mar 17, 2024

  1. I'm not sure, I'd have to check. @AlexWaygood added that feature, I presume there's a way to unset keys.
  2. I don't have a strong preference. options.to_ruff_args() + overrides.to_ruff_args() seems reasonable to start.

@hoel-bagard hoel-bagard linked a pull request Mar 17, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ci Related to internal CI tooling help wanted Contributions especially welcome
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

replace config_overrides.patch_config by config_overrides.to_ruff_args hoel-bagard/ruff
2 participants
@zanieb @hoel-bagard