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

braid diff no longer pulls the mirror if braided version is not present in cache #69

Closed
realityforge opened this issue Jul 15, 2017 · 2 comments
Closed

braid diff no longer pulls the mirror if braided version is not present in cache #69

realityforge opened this issue Jul 15, 2017 · 2 comments
Assignees
@mattmccutchen

Comments

@realityforge
Copy link
Collaborator

If the version braided into a project is not present in the local cache, braid should synchronize the mirror and try again. Historically it used to always synchronize the mirror so this was never a problem.
@realityforge realityforge self-assigned this Jul 15, 2017
@mattmccutchen mattmccutchen mentioned this issue Sep 25, 2017
Open
@mattmccutchen
Copy link
Collaborator

Oops, this is a mistake in #62. The minimal fix to restore the behavior before #62 would be to add the missing mirror.fetch call, but due to #71, I propose to check whether the upstream revision is present in the repository before fetching, as you suggest. This does not address the underlying performance problem, but at least it avoids the problem for repeated braid diff calls and doesn't add any complexity that will hurt us later. We can always implement something better later.

@realityforge
Copy link
Collaborator Author

Indeed - that seems like a reasonable approach. I would love to see this one fixed. It bites me ... every now and again ;)

mattmccutchen added a commit to mattmccutchen/braid that referenced this issue Dec 27, 2017
@mattmccutchen
Before diffing a mirror, fetch the base revision if the downstream
89bc7e9 
repository doesn't have it.

Fixes cristibalan#69.
@mattmccutchen mattmccutchen mentioned this issue Dec 27, 2017
Merged
mattmccutchen added a commit to mattmccutchen/braid that referenced this issue Dec 27, 2017
@mattmccutchen
Before diffing a mirror, fetch the base revision if the downstream
2e4748a 
repository doesn't have it.

Remove the mock-based tests of Mirror#diff.  They would need to be
updated, and I don't think it's worth continuing to maintain them when
all callers are covered by integration tests.

Fixes cristibalan#69.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattmccutchen mattmccutchen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@realityforge @mattmccutchen