Skip to content

checkout: Fix bare-user symlink checkouts#799

Closed
cgwalters wants to merge 1 commit intoostreedev:masterfrom
cgwalters:bareuser-co-regression
Closed

checkout: Fix bare-user symlink checkouts#799
cgwalters wants to merge 1 commit intoostreedev:masterfrom
cgwalters:bareuser-co-regression

Conversation

@cgwalters
Copy link
Copy Markdown
Member

Logic error introduced after refactoring; we hoisted the
is_bare_user_symlink variable to the top, but its computation
below. But the is_bare symlink depended on it.

Closes: #798

Logic error introduced after refactoring; we hoisted the
`is_bare_user_symlink` variable to the top, but its computation
below.  But the `is_bare` symlink depended on it.

Closes: ostreedev#798
@alexlarsson
Copy link
Copy Markdown
Member

Looks good to me.

@alexlarsson
Copy link
Copy Markdown
Member

Maybe we should have a symlink in our basic tests though?

@cgwalters
Copy link
Copy Markdown
Member Author

We did have one, we just weren't verifying it. That's the addition to the test suite I made. Right?

@alexlarsson
Copy link
Copy Markdown
Member

Oh, missed that part. Yeah, that looks good.

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Apr 18, 2017

Ouch! Just to sanity check, I verified that the new symlink check would have failed without this fix.

@rh-atomic-bot r+ ee918d9

@rh-atomic-bot
Copy link
Copy Markdown

⌛ Testing commit ee918d9 with merge 08964d5...

@rh-atomic-bot
Copy link
Copy Markdown

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing 08964d5 to master...

cgwalters added a commit that referenced this pull request Apr 18, 2017
Logic error introduced after refactoring; we hoisted the
`is_bare_user_symlink` variable to the top, but its computation
below.  But the `is_bare` symlink depended on it.

Closes: #798

Closes: #799
Approved by: jlebon
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants