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

Query realdep and index improvement #696

Closed

Conversation

j-mracek
Copy link
Contributor

No description provided.

@ignatenkobrain
Copy link
Contributor

I still think returning an error is much better than silently skipping it.

if (reldeplist->count()) {
pImpl->filters.push_back(Filter(keyname, HY_EQ, reldeplist));
} else {
addFilter(HY_PKG_EMPTY, HY_EQ, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not pImpl->filters.push_back(Filter(HY_PKG_EMPTY, HY_EQ, 1)); ?

If provided string to a query is incorrectly formatted, the query should
be empty.

https://bugzilla.redhat.com/show_bug.cgi?id=1687135
The formal implementation results in Segmentation fault (core dumped).
@jrohel
Copy link
Contributor

jrohel commented Mar 13, 2019

@ignatenkobrain Yes, hiding errors can bring problems later.
But is it an error in this case? Ok, it seems as syntax error, but on the other hand, the user gets what he requested -> nonexistent dependency = empty list.
It is consistent with "glob" search now. This also returns empty list:
dnf repoquery --whatprovides '(crate(pkg-c*onfig/default)'
It is about point of view.

@jrohel
Copy link
Contributor

jrohel commented Mar 13, 2019

@rh-atomic-bot r+

@rh-atomic-bot
Copy link

📌 Commit ae2f46a has been approved by jrohel

@rh-atomic-bot
Copy link

⌛ Testing commit ae2f46a with merge 45d973a...

rh-atomic-bot pushed a commit that referenced this pull request Mar 13, 2019
The formal implementation results in Segmentation fault (core dumped).

Closes: #696
Approved by: jrohel
@rh-atomic-bot
Copy link

☀️ Test successful - status-papr
Approved by: jrohel
Pushing 45d973a to master...

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