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

[cmd diff] Run names with literal : in them understood differently than store and cmd runs commands #3535

Closed
whisperity opened this issue Dec 6, 2021 · 5 comments · Fixed by #3536
Assignees
Labels
CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands enhancement 🌟

Comments

@whisperity
Copy link
Contributor

whisperity commented Dec 6, 2021

Describe the bug

If the run name is in some way "complex", cmd diff constantly reports No run names with the given pattern:. However, the exact same format of run name is accepted by cmd results (and also store).

The complex run-name in question looks like this: my/complex-run: name. (Auto-generated from Git user/repository: branch format.)

CodeChecker version

612b726

To Reproduce

  1. store --name simple-1
  2. store --name simple-2
  3. store --name "my/complex-run: name"
  4. cmd results "my/complex-run: name" works. ✔️
  5. cmd diff --new --basename "simple-1" --newname "simple-2" works. ✔️
  6. cmd diff --new --basename "my/complex-run: name" --newname "simple-1" doesn't work. ❌

Expected behaviour

The run name is accepted, and before pattern-matching(?) or file system traversal, exact string match is done for the remote name. (In addition, if there are BOTH a local directory structure AND a remote run with the same pattern(?), a warning is emitted.)

System

  • OS: Ubuntu 18.04

Additional context

The issue appears both when diffing remote to local and remote to remote, and in both directions.

If I make a directory locally, and execute diff, it will pick the directory up as the local run. That is, if the following directory structure appears locally (even if empty) ...

$ tree my
my
└── complex-run: name

... then all the remote findings will be regarded as --new, and reported.

I tried escaping most of the suspicious characters in the run name, i.e. "my\/complex-run: name", "my\/complex\-run\:\ name", and various other combinations of escape, but had no luck.

The exact error message is a bit misleading, I had to have the hunch to try making a directory, because the following relevant lines are printed, even with --verbose debug.

[INFO][2021-12-06 11:25:31] {system} [14277] <140079117096768> - cmd_line_client.py:1190 handle_diff_results() - Given remote runs (--baseline): my/complex-run: name
[DEBUG][2021-12-06 11:25:31] {system} [14277] <140079117096768> - client.py:257 setup_client() - Initializing client connecting to [...] done.
[WARNING][2021-12-06 11:25:32] {system} [14277] <140079117096768> - cmd_line_client.py:156 process_run_args() - No run names match the given pattern: my/complex-run: name

It does not even say that it is trying to find local directory structures.

@whisperity whisperity added bug 🐛 CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands labels Dec 6, 2021
@whisperity whisperity changed the title [cmd] Seemingly "complex" run names fail to match(?) when diffing [cmd diff] Run names with / in them unconditionally attempted as local directory Dec 6, 2021
@csordasmarton
Copy link
Contributor

@whisperity The client thinks that you gave complex-run as the run name and name as the history tag name:

run_with_tag = run_arg_with_tag.split(':')

@whisperity
Copy link
Contributor Author

The client thinks that you gave complex-run as the run name and name as the history tag name

@csordasmarton Ah, thank you. Then it seems local directories do indeed take priority!

Would it be possible to implement some sort of an "escape" mechanism here, e.g. first match for \:, then do not split, and rewrite as :? The issue is that other commands happily accept : as-is, without trying to understand it as a tag. (store has a dedicated --run-tag or somesuch CLI option, I'm not sure about cmd results.)

Or should I just come up with a better run name generating logic, and add something to my documentation to discourage users from putting : into their run names (if they configure it themselves)?

@csordasmarton
Copy link
Contributor

csordasmarton commented Dec 6, 2021

I think we can support escaping here, similar to this:

>>> run_name="my/comple-run\: name:tag_version"
>>> list(map(lambda x: x.replace("\:", ":"), re.split(r"(?<!\\):", run_name)))
['my/comple-run: name', 'tag_version']

@whisperity
Copy link
Contributor Author

@csordasmarton Perfect! 😃 Will you have some time to add this small feature, or should I?

@csordasmarton
Copy link
Contributor

If you have time, please create a PR for it. Also don't forget to add a test case for it 😊

@whisperity whisperity self-assigned this Dec 6, 2021
@whisperity whisperity changed the title [cmd diff] Run names with / in them unconditionally attempted as local directory [cmd diff] Run names with literal : in them understood differently than store and cmd runs commands Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands enhancement 🌟
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants