select --from tries to select packages from a different staging as well

[Vogtinator]

With xdg-desktop-portal-kde in an adi staging, osc staging select --from D K xdg-desktop-portal-kde tries to move it as well:
(1/1) Moving "636539" from "openSUSE:Factory:Staging:D" to "openSUSE:Factory:Staging:K"

It doesn't actually move the request though AFAICT.

[jberry-suse]

Based on the documentation that is an invalid command. Personally, I never use --from as the the package name / request number should be enough to identify a what is to be moved. Any duplicates should be handled already.

osc staging select [--no-freeze] [--move [--from STAGING]]

In general, the following should work:

osc staging select --move K xdg-desktop-portal-kde

I am going to assume the command you included is not what actually ran, otherwise I'd have to stare at this code a bit more to see how it passed the following condition.

elif request in staged_requests and (move or supersede):

Additionally, request 636539 is for the package in your command: xdg-desktop-portal-kde. So it seems to have selected the appropriate package not xdg-desktop-portal.

Either I am missing something or this report either needs more information or has some incorrect information in it.

[Vogtinator]

That's literally what I ran and what the output was. xdg-desktop-portal-kde was and still is in the adi staging.

Additionally, request 636539 is for the package in your command: xdg-desktop-portal-kde. So it seems to have selected the appropriate package not xdg-desktop-portal.

That was a typo, I meant xdg-desktop-portal-kde.

[jberry-suse]

To be clear, you wrote:

osc staging select --from D K xdg-desktop-portal-kde

I wrote:

osc staging select --move K xdg-desktop-portal-kde

I am going to assume you ran:

osc staging select --move --from D K xdg-desktop-portal-kde

The devil is in the details. From what you wrote:

• It failed to move request 636539 from staging K which seems valid since the request was not in K.

Again, the specifics here matter.

[Vogtinator]

Right, another typo. This time copy and pasted: HOME=~/favogt_factory osc staging select --move --from D K xdg-desktop-portal-kde

It failed to move request 636539 from staging K which seems valid since the request was not in K.

It did not fail - it had as output that the request was moved but nothing happened.

[jberry-suse]

Alright, so the message is wrong in that it should say failed to find request in K or something along those lines? In the future can you just skip --from? I would vote for removing the option entirely as a the request ID can be used if you really need to be specific.

[Vogtinator]

Alright, so the message is wrong in that it should say failed to find request in K or something along those lines?

I guess so.

In the future can you just skip --from? I would vote for removing the option entirely as a the request ID can be used if you really need to be specific.

Use case for the call was to move all Plasma packages from D to K, but the list of package names given also included packages which are part of adi stagings.

[jberry-suse]

--from does not operate as a wildcard select although that would actually make more sense. It operates as an additional constraint on where to look for the specified requests to move. So it looks for xdg-desktop-portal-kde in D instead of the adi where it is and thus fails to move it without printing such a message.

[jberry-suse]

Given that it can find the requests where ever they are staged I am going to assume there was some legacy reason for --from that as far as I can tell no longer makes sense.

[Vogtinator]

--from does not operate as a wildcard select although that would actually make more sense. It operates as an additional constraint on where to look for the specified requests to move. So it looks for xdg-desktop-portal-kde in D instead of the adi where it is and thus fails to move it without printing such a message.

Given that it can find the requests where ever they are staged I am going to assume there was some legacy reason for --from that as far as I can tell no longer makes sense.

It makes sense as such an additional constraint though and apparently works fine - just the message needs to be improved.

[jberry-suse]

But why would you need it if you have explicitly list all the packages you want to move anyway? It can figure out that they come from D and adi automatically. Based on what you describe it would have worked perfectly if you dropped --from D and left the rest as it was.

[Vogtinator]

The point is that I did not want to move those which are in an adi staging.

[jberry-suse]

And based on the various checks and design...one should never have multiple requests staged for the same package. As such a list of either package names or request ids is already unique and does not require a --from.

[jberry-suse]

The point is that I did not want to move those which are in an adi staging.

I am baffled it took this much to get the one sentence that should have been in the original description. This is an entirely different discussion...using it as an input filter.

[Vogtinator]

I am baffled it took this much to get the one sentence that should have been in the original description.

I thought it was sufficiently explained in the issue title... Apparently not, sorry for that.

[jberry-suse]

select --from tries to select packages from a different staging as well

The sentence is correct, but does not describe your intent at all. Sounds more like an issue in that it doesn't actually move the request like you think it should. Furthered by, "It doesn't actually move the request though AFAICT."

The original code was design as an aid in the way I already described. There was no filtering and the additional stuff I added is completely separate.

openSUSE-release-tools/osclib/select_command.py

Lines 76 to 81 in 5a2cd2d

	fprj = None
	if from_:
	fprj = self.api.prj_from_letter(from_)
	else:
	# supersede = (new_rq, package, project)
	fprj = self.api.packages_staged[staged_requests[request]]['prj'] if not supersede else supersede[2]

As such it would be best to fully understand the use-case before deciding how we might proceed.

• If you want just to move all from staging D to K that seems like either a new syntax/command/etc.
• If you want to specifically be able to filter a list of requests based on the staging they are in currently and move only the ones that meet that condition...could alter to work that way.

I assume if you are filtering this has something to do with having a full list of kde related packages and just wanting to move the ones in D? Somewhat related perhaps towards #731.

The current flag serves no purpose as far as I can tell.

[Vogtinator]

I assume if you are filtering this has something to do with having a full list of kde related packages and just wanting to move the ones in D?

Yup!

Somewhat related perhaps towards #731.

Maybe - only that an interactive window doesn't make it easily possible to use a list of packages in a file as selection:

<plasma/pkglist-5.14 osc staging select --move --from D K

[jberry-suse]

Well, I am not opposed to changing the behavior to print a message there that ignoring since not matching --from or similar. Perhaps just dropping the old and replacing with --filter-from or some such.

Untest, but you can try if you want since sounds like you use this.

diff --git a/osclib/select_command.py b/osclib/select_command.py
index 88edf6e3..b690e729 100644
--- a/osclib/select_command.py
+++ b/osclib/select_command.py
@@ -73,12 +73,11 @@ class SelectCommand(object):
             return self.api.rq_to_prj(request, self.target_project)
         elif request in staged_requests and (move or supersede):
             # 'select' command becomes a 'move'
-            fprj = None
-            if from_:
-                fprj = self.api.prj_from_letter(from_)
-            else:
-                # supersede = (new_rq, package, project)
-                fprj = self.api.packages_staged[staged_requests[request]]['prj'] if not supersede else supersede[2]
+            # supersede = (new_rq, package, project)
+            fprj = self.api.packages_staged[staged_requests[request]]['prj'] if not supersede else supersede[2]
+            if from_ != fprj:
+                print('Ignoring "{}" in "{}" since not "{}"'.format(request, fprj, from_))
+                return True
 
             if supersede:
                 print('"{} ({}) is superseded by {}'.format(request, supersede[1], supersede[0]))
@@ -133,6 +132,7 @@ class SelectCommand(object):
 
         requests = RequestFinder.find_sr(requests, self.api, newcand)
         requests_count = len(requests)
+        from_ = self.api.prj_from_letter(from_)
         for index, request in enumerate(requests, start=1):
             print('({}/{}) '.format(index, requests_count), end='')
             if not self.select_request(request, move, from_):

[Vogtinator]

That diff seems to work (ignoring the missing "in"):

(1/1) Ignoring "636539" in "openSUSE:Factory:Staging:adi:14" since not "openSUSE:Factory:Staging:D"