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

Error messages when merging structure-incompatible files #3745

Open
Richargh opened this issue Sep 16, 2024 · 2 comments · May be fixed by #3774
Open

Error messages when merging structure-incompatible files #3745

Richargh opened this issue Sep 16, 2024 · 2 comments · May be fixed by #3774
Assignees
Labels
difficulty:medium The difficulty to solve this is not super complex but not easy either pr-analysis Issues that touch the analysis pr(oject). priority:high Set by PO

Comments

@Richargh
Copy link

Feature request

Description

As an auditor, I want merge to be aborted with a descriptive error message when the file&folder structure does not match, so the merged file is always valid.

Context

Suppose I have a project with this structure:

├📁 mailbox
├─📁 src/
├─📄 pom.xml
├─📄 Readme.adoc
├─📄 sonar.properties

The mailbox.git.cc.json then mirrors this structure:

├📁 root
├─📁 src/
├─📄 pom.xml
├─📄 Readme.adoc
├─📄 sonar.properties

but the mailbox.sonar.cc.json might look like:

├📁 root
├─📁 mailbox/
├──📁 src/
├──📄 pom.xml 
# Readme and sonar.properties are missing because sonar does not analyse them

This behavior has been noticed in some cases for the sonar parser, but is also found in the tokei parser and in the raw text parser depending on how code is analysed.

If these files are merged the result will be very strange:

├📁 root
├─📁 mailbox/
├──📁 src/
├──📄 pom.xml 
├─📁 src/
├─📄 pom.xml
├─📄 Readme.adoc
├─📄 sonar.properties

This is clearly and error and merge should abort when it notices such a situation.
Right now one can diagnose this problem by running ccsh modify -p 1 prj.x.cc.json and comparing the structure inside the cc.json to other cc.json structures.
This is time-consuming when one has to merge 20+ files.
The bigger problem is that you don't notice the problem and think it was merged correctly.

The feature request is thus that merge looks at the top-level structure of the .cc.jsons to be merged, only the first level and not recursive.
When one of the .cc.json has a file&folder structure that does not have overlap with the other files, then the merge is aborted.

Acceptance criteria

  • ccsh merge prj.x.cc.json prj.y.cc.json prj.z.cc.json fails if the structure of at least one file has no overlap with one of the other files. For example:
    • When one prj.x.cc.json has the contents foo/, bar/, baz/ and prj.y.cc.json has foo/, trick/, track/ then these files have an overlap of one folder and are merged.
    • When one prj.x.cc.json has the contents foo/, bar/, one.xml and prj.y.cc.json has trick/, track/, one.xml then these files have an overlap of one file and are merged.
    • When one prj.x.cc.json has the contents foo/, bar/, one.xml and prj.y.cc.json has trick/, track/, two.xml then these files have no overlap and are not merged with an error message.
    • This is not a recursive test. Only the first level counts.
  • In case of structure mismatch, the error message prints a couple of files from the first level of the offending cc.json and then quits.
  • A new command-line option -f exists to still merge the files together
  • The structure-overlap check is not performed when running the fat merge from Merge multiple projects files into one fat .cc.json #3743

Assumptions & Exclusions

  • This is not fool-proof. It does however provide a better safety-net when merging files and avoids a common error.

Development notes (optional Task Breakdown)

  • [ ]
  • [ ]
  • [ ]

Open questions

@ChristianHuehn ChristianHuehn added priority:high Set by PO pr-analysis Issues that touch the analysis pr(oject). difficulty:medium The difficulty to solve this is not super complex but not easy either labels Sep 19, 2024
@ViktoriaGordeevaVG ViktoriaGordeevaVG self-assigned this Sep 24, 2024
@phanlezz
Copy link
Collaborator

similar to #3495

@ViktoriaGordeevaVG
Copy link
Collaborator

These are currently two options to check for overlaps between the modules before merging them.

First, you can use the command directly by entering, for example, ccsh merge file_1x.cc.json file_2.cc.json ... file_n.cc.json. It will then immediately check if there is at least one overlap between all the files. If not, the process will be aborted and terminated. To ensure that the merge still happens, you can add the -f flag: ccsh merge file_1x.cc.json file_2.cc.json ... file_n.cc.json -f.

The second option is through the DialogParser, by specifying the folder containing all the cc.json files. In this case, the DialogParser will go through all the questions, and if there are no overlaps between the files, the user will be asked again if they still want to merge the modules.

ViktoriaGordeevaVG pushed a commit that referenced this issue Sep 30, 2024
ViktoriaGordeevaVG pushed a commit that referenced this issue Sep 30, 2024
@ViktoriaGordeevaVG ViktoriaGordeevaVG linked a pull request Sep 30, 2024 that will close this issue
6 tasks
ViktoriaGordeevaVG pushed a commit that referenced this issue Oct 1, 2024
ViktoriaGordeevaVG pushed a commit that referenced this issue Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty:medium The difficulty to solve this is not super complex but not easy either pr-analysis Issues that touch the analysis pr(oject). priority:high Set by PO
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants