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

fix: stop deletion of contents if directory paths overlap #325

Closed

Conversation

fritzduchardt
Copy link
Contributor

@fritzduchardt fritzduchardt commented Dec 3, 2023

/closes #323

@kumaritanushree
Copy link
Contributor

@fritzduchardt can you please add issue if there is any to this MR or add more description why you have added this code and what issue you have faced?

@fritzduchardt fritzduchardt force-pushed the fix-overlapping-paths branch 2 times, most recently from f71e2b7 to 03a7488 Compare December 4, 2023 08:52
Signed-off-by: Fritz Duchardt <fritz@duchardt.net>
@fritzduchardt
Copy link
Contributor Author

@fritzduchardt can you please add issue if there is any to this MR or add more description why you have added this code and what issue you have faced?

Sure @kumaritanushree, I have linked the issue (and fixed the pipeline)

@kumaritanushree
Copy link
Contributor

@fritzduchardt for your use case can you please try this config:

kind: Config
directories:
- path: vendor
  contents:
  - path: dir1/output
    directory:
      path: ./local-dir
  - path: dir2/output
    directory:
      path: ./local-dir-2

What I know is vendir is designed to manage each directory separately. In your example you are telling vendir to manage the same directory “vendor” in 2 different directives.

I think this should resolve your problem. And for better user experience I think we can add error with suggestion(if possible) for overlapping path.

@fritzduchardt
Copy link
Contributor Author

@fritzduchardt for your use case can you please try this config:

kind: Config
directories:
- path: vendor
  contents:
  - path: dir1/output
    directory:
      path: ./local-dir
  - path: dir2/output
    directory:
      path: ./local-dir-2

What I know is vendir is designed to manage each directory separately. In your example you are telling vendir to manage the same directory “vendor” in 2 different directives.

I think this should resolve your problem. And for better user experience I think we can add error with suggestion(if possible) for overlapping path.

I feel that at the very least there should be an overlap warning. Still, let's consider supporting the vendir.yaml structure that I provided, since there is no factual overlap and it gives the user more flexibility. Let's discuss in the next community meeting.

@kumaritanushree
Copy link
Contributor

I feel that at the very least there should be an overlap warning.

@fritzduchardt
yeah we can add overlap warning for better understanding of flow.

Still, let's consider supporting the vendir.yaml structure that I provided, since there is no factual overlap and it gives the user more flexibility. Let's discuss in the next community meeting.

Sure, we can discuss this in community, we will get more thoughts of people there and can decide accordingly.

@Zebradil
Copy link
Member

I bumped into a similar issue and proposed a fix in #343. I was sure that vendir didn't allow to have multiple directories with the same path, but I was unable to find the corresponding code in git history.

@kumaritanushree
Copy link
Contributor

@fritzduchardt do you want to close this PR as we decided to not go with this approach and @Zebradil is already working on approach what we have decided in community in this PR

@Zebradil Zebradil deleted the fix-overlapping-paths branch February 12, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Vendir directory sync ignores overlapping paths on directories
3 participants