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

Don't delete refs having aliases #1749

Closed
wants to merge 1 commit into from

Conversation

sinnykumari
Copy link
Collaborator

Deleting a ref with aliases makes them dangling. In such
cases, display an error message to the user.

Fixes #1597

Signed-off-by: Sinny Kumari sinny@redhat.com

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor bits.

ref_alias, const char *, value)
{
if (!strcmp(ref, value))
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks off here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, fixed it!
Is there an automatic source formatter (config) which I can use during OSTree development?

if (!strcmp(ref, value))
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Ref %s has some aliases, first delete corresponding aliases", ref);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about: "Ref '%s' has an active alias: '%s'", ref, ref_alias

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, updated the error message.

@sinnykumari
Copy link
Collaborator Author

This PR has been updated with changes from feedback and ready for further review.

GLNX_HASH_TABLE_FOREACH_KV (ref_aliases, const char *,
ref_alias, const char *, value)
{
if (!strcmp(ref, value))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing space between strcmp and '('

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed it!

Deleting a ref with aliases makes them dangling. In such
cases, display an error message to the user.

Fixes ostreedev#1597

Signed-off-by: Sinny Kumari <sinny@redhat.com>
@cgwalters
Copy link
Member

@rh-atomic-bot r+ d84e50a

@rh-atomic-bot
Copy link

⌛ Testing commit d84e50a with merge a125586...

rh-atomic-bot pushed a commit that referenced this pull request Oct 10, 2018
Deleting a ref with aliases makes them dangling. In such
cases, display an error message to the user.

Fixes #1597

Signed-off-by: Sinny Kumari <sinny@redhat.com>

Closes: #1749
Approved by: cgwalters
@dustymabe
Copy link
Contributor

Nice work @sinnykumari !

@rh-atomic-bot
Copy link

💥 Test timed out

@cgwalters
Copy link
Member

@rh-atomic-bot retry

@rh-atomic-bot
Copy link

⌛ Testing commit d84e50a with merge c705268...

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing c705268 to master...

@sinnykumari
Copy link
Collaborator Author

Thanks @cgwalters !

jlebon added a commit to jlebon/ostree that referenced this pull request Oct 30, 2018
This is the alias version of ostreedev#1749. I.e. we want to make sure that one
can't even create an alias which would end up dangling.

See also: https://pagure.io/releng/issue/7891
rh-atomic-bot pushed a commit that referenced this pull request Oct 31, 2018
This is the alias version of #1749. I.e. we want to make sure that one
can't even create an alias which would end up dangling.

See also: https://pagure.io/releng/issue/7891

Closes: #1768
Approved by: sinnykumari
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.

None yet

5 participants