)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"d84f5456f4a135941cff19a8c3f511cc2fedec2c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f926cb9f_2a6687c5","updated":"2023-08-31 03:53:58.000000000","message":"recheck","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"}],"diskimage_builder/elements/package-installs/bin/package-installs-squash":[{"author":{"_account_id":30491,"name":"Radosław Piliszek","display_name":"Radek","email":"radek@piliszek.it","username":"yoctozepto","status":"self-employed techologist, collaborating mostly with 7bulls.com"},"change_message_id":"65338b8e093b0639713d0cc23e401de3c9a5bf3a","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            phase \u003d param.get(\u0027phase\u0027, \u0027install.d\u0027)"},{"line_number":130,"context_line":"            installs \u003d [\"install\"]"},{"line_number":131,"context_line":"            if \u0027uninstall\u0027 in param or \u0027build-only\u0027 in param:"},{"line_number":132,"context_line":"                # We don\u0027t add the package to the uninstall list as"},{"line_number":133,"context_line":"                # something else has requested we install it without"},{"line_number":134,"context_line":"                # removing it."},{"line_number":135,"context_line":"                in_install \u003d any("}],"source_content_type":"application/octet-stream","patch_set":2,"id":"9f560f44_4dce6b5a","line":132,"range":{"start_line":132,"start_character":65,"end_line":132,"end_character":67},"updated":"2020-08-21 07:36:29.000000000","message":"nit: if","commit_id":"75027cf664d8baad50ebe518ae11e9f6add6bee1"},{"author":{"_account_id":30491,"name":"Radosław Piliszek","display_name":"Radek","email":"radek@piliszek.it","username":"yoctozepto","status":"self-employed techologist, collaborating mostly with 7bulls.com"},"change_message_id":"65338b8e093b0639713d0cc23e401de3c9a5bf3a","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                    map(lambda x: x[0] \u003d\u003d pkg_name, data[phase][\"install\"]))"},{"line_number":137,"context_line":"                not_in_uninstall \u003d all("},{"line_number":138,"context_line":"                    map(lambda x: x[0] !\u003d pkg_name, data[phase][\"uninstall\"]))"},{"line_number":139,"context_line":"                if in_install and not_in_uninstall and \u0027build-only\u0027 in param:"},{"line_number":140,"context_line":"                    # Add to installs list for completeness in the build-only"},{"line_number":141,"context_line":"                    # case."},{"line_number":142,"context_line":"                    installs \u003d [\"install\"]"},{"line_number":143,"context_line":"                elif in_install and not_in_uninstall:"},{"line_number":144,"context_line":"                    # Just skip further processing as we have no uninstall"},{"line_number":145,"context_line":"                    # work to do"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"9f560f44_adf0e792","line":142,"range":{"start_line":139,"start_character":16,"end_line":142,"end_character":42},"updated":"2020-08-21 07:36:29.000000000","message":"it\u0027s the default above","commit_id":"75027cf664d8baad50ebe518ae11e9f6add6bee1"},{"author":{"_account_id":30491,"name":"Radosław Piliszek","display_name":"Radek","email":"radek@piliszek.it","username":"yoctozepto","status":"self-employed techologist, collaborating mostly with 7bulls.com"},"change_message_id":"12d462af1d6984c45e255ea8c3b408d2ed74803c","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                not_in_uninstall \u003d all("},{"line_number":138,"context_line":"                    map(lambda x: x[0] !\u003d pkg_name, data[phase][\"uninstall\"]))"},{"line_number":139,"context_line":"                if in_install and not_in_uninstall:"},{"line_number":140,"context_line":"                    if \u0027build-only\u0027 not in param:"},{"line_number":141,"context_line":"                        # Just skip further processing as we have no uninstall"},{"line_number":142,"context_line":"                        # work to do"},{"line_number":143,"context_line":"                        continue"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_71c98157","line":140,"range":{"start_line":140,"start_character":23,"end_line":140,"end_character":42},"updated":"2020-08-21 15:53:12.000000000","message":"this reads better as:\nif \u0027uninstall\u0027 in param\n\nI wonder if it shouldn\u0027t have actually been just: \u0027build-only\u0027 in param\n\nor without this condition at all - just make all permanent installs permanent","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"ddae6a1372a7bbcbe17c42a42da86fd0798c5ab3","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                not_in_uninstall \u003d all("},{"line_number":138,"context_line":"                    map(lambda x: x[0] !\u003d pkg_name, data[phase][\"uninstall\"]))"},{"line_number":139,"context_line":"                if in_install and not_in_uninstall:"},{"line_number":140,"context_line":"                    if \u0027build-only\u0027 not in param:"},{"line_number":141,"context_line":"                        # Just skip further processing as we have no uninstall"},{"line_number":142,"context_line":"                        # work to do"},{"line_number":143,"context_line":"                        continue"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_ac42a0a9","line":140,"range":{"start_line":140,"start_character":23,"end_line":140,"end_character":42},"in_reply_to":"9f560f44_0cd3cc61","updated":"2020-08-21 16:41:00.000000000","message":"It preserves the \"install\" side of the old behavior. The old datastructure we have is something like:\n\n  packages:\n    install:\n      - (git, element1)\n      - (vim, element1)\n      - (git, element2)\n    uninstall:\n      - (emacs, element1)\n      - (git, element2)\n\nWhat I am trying to preserve here is that (git, element2) remains in the install list even if it isn\u0027t in the uninstall list. That said I do think you are right that this isn\u0027t strictly necessary as git is already present and we know we don\u0027t want to uninstall it.\n\nI\u0027m not familiar enough with the package installs element to know how important the element by element listing even for duplicate packages is. If it isn\u0027t very important then I agree we can probably make the change you suggest.","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"},{"author":{"_account_id":30491,"name":"Radosław Piliszek","display_name":"Radek","email":"radek@piliszek.it","username":"yoctozepto","status":"self-employed techologist, collaborating mostly with 7bulls.com"},"change_message_id":"b7240db974fccc1c9f396b658baddf0caf7aaaf6","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                not_in_uninstall \u003d all("},{"line_number":138,"context_line":"                    map(lambda x: x[0] !\u003d pkg_name, data[phase][\"uninstall\"]))"},{"line_number":139,"context_line":"                if in_install and not_in_uninstall:"},{"line_number":140,"context_line":"                    if \u0027build-only\u0027 not in param:"},{"line_number":141,"context_line":"                        # Just skip further processing as we have no uninstall"},{"line_number":142,"context_line":"                        # work to do"},{"line_number":143,"context_line":"                        continue"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_0cd3cc61","line":140,"range":{"start_line":140,"start_character":23,"end_line":140,"end_character":42},"in_reply_to":"9f560f44_7174e1cf","updated":"2020-08-21 16:21:20.000000000","message":"Well, it does not really preserve the old behaviour, as that would be [\"install\", \"uninstall\"]. I still argue that it reads better as I wrote above as this is an equivalent condition under the common \u0027if\u0027. Or even without this condition.","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"9b48ad3b9caa3c99dfe9553863f23b1026d6cfdf","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                not_in_uninstall \u003d all("},{"line_number":138,"context_line":"                    map(lambda x: x[0] !\u003d pkg_name, data[phase][\"uninstall\"]))"},{"line_number":139,"context_line":"                if in_install and not_in_uninstall:"},{"line_number":140,"context_line":"                    if \u0027build-only\u0027 not in param:"},{"line_number":141,"context_line":"                        # Just skip further processing as we have no uninstall"},{"line_number":142,"context_line":"                        # work to do"},{"line_number":143,"context_line":"                        continue"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_7174e1cf","line":140,"range":{"start_line":140,"start_character":23,"end_line":140,"end_character":42},"in_reply_to":"9f560f44_71c98157","updated":"2020-08-21 16:01:04.000000000","message":"The reason this is \u0027build-only\u0027 not in param is that the old code checked both and not exclusively. That means we only want to do a full skip if build-only is not set. If uninstall and build-only are set we don\u0027t want to skip and want to process with just installs \u003d [\u0027install\u0027] to preserve the old behavior.","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"},{"author":{"_account_id":30491,"name":"Radosław Piliszek","display_name":"Radek","email":"radek@piliszek.it","username":"yoctozepto","status":"self-employed techologist, collaborating mostly with 7bulls.com"},"change_message_id":"82fdd7f852768fbc6d43be5e415d665bc86fb36a","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                not_in_uninstall \u003d all("},{"line_number":138,"context_line":"                    map(lambda x: x[0] !\u003d pkg_name, data[phase][\"uninstall\"]))"},{"line_number":139,"context_line":"                if in_install and not_in_uninstall:"},{"line_number":140,"context_line":"                    if \u0027build-only\u0027 not in param:"},{"line_number":141,"context_line":"                        # Just skip further processing as we have no uninstall"},{"line_number":142,"context_line":"                        # work to do"},{"line_number":143,"context_line":"                        continue"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_acde20d7","line":140,"range":{"start_line":140,"start_character":23,"end_line":140,"end_character":42},"in_reply_to":"9f560f44_ac42a0a9","updated":"2020-08-21 17:19:52.000000000","message":"OK, you want to preserve duplicate installs, as in the new code the above \"old\" would become (with current code):\n\n  packages:\n    install:\n      - (git, element1)\n      - (vim, element1)\n      - (git, element2)\n    uninstall:\n      - (emacs, element1)\n\nas opposed to:\n\n  packages:\n    install:\n      - (git, element1)\n      - (vim, element1)\n    uninstall:\n      - (emacs, element1)\n\nIt is actually kind of important, yes, as elements have their own pkg maps.\nIt would be confusing to see them apply different maps but that is a problem for another day. :-)","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"},{"author":{"_account_id":30491,"name":"Radosław Piliszek","display_name":"Radek","email":"radek@piliszek.it","username":"yoctozepto","status":"self-employed techologist, collaborating mostly with 7bulls.com"},"change_message_id":"12d462af1d6984c45e255ea8c3b408d2ed74803c","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                        installs \u003d [\"install\", \"uninstall\"]"},{"line_number":149,"context_line":"            else:"},{"line_number":150,"context_line":"                # Remove any uninstallations if we are trying to install"},{"line_number":151,"context_line":"                # the package without uninstallation elsewhere."},{"line_number":152,"context_line":"                data[phase][\"uninstall\"] \u003d ["},{"line_number":153,"context_line":"                    x for x in data[phase][\"uninstall\"] if x[0] !\u003d pkg_name]"},{"line_number":154,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_31370950","line":151,"range":{"start_line":151,"start_character":53,"end_line":151,"end_character":62},"updated":"2020-08-21 15:53:12.000000000","message":"\u0027here\u0027 I guess; it must have been tried \u0027elsewhere\u0027 if we can remove it\n\nIs this branch for optimisation?","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"9b48ad3b9caa3c99dfe9553863f23b1026d6cfdf","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                        installs \u003d [\"install\", \"uninstall\"]"},{"line_number":149,"context_line":"            else:"},{"line_number":150,"context_line":"                # Remove any uninstallations if we are trying to install"},{"line_number":151,"context_line":"                # the package without uninstallation elsewhere."},{"line_number":152,"context_line":"                data[phase][\"uninstall\"] \u003d ["},{"line_number":153,"context_line":"                    x for x in data[phase][\"uninstall\"] if x[0] !\u003d pkg_name]"},{"line_number":154,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_b1a5d949","line":151,"range":{"start_line":151,"start_character":53,"end_line":151,"end_character":62},"in_reply_to":"9f560f44_31370950","updated":"2020-08-21 16:01:04.000000000","message":"No this is required beause we don\u0027t control the order or processing of the individual package files. The two test cases added in this change try to capture that as well.\n\nWe have the case where we try to set build-only or uninstall on a package that is already explicitly only installed (thats the first block). Then we have the second case where something set build-only or uninstall then we find a second case that is install only (this case).","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"},{"author":{"_account_id":30491,"name":"Radosław Piliszek","display_name":"Radek","email":"radek@piliszek.it","username":"yoctozepto","status":"self-employed techologist, collaborating mostly with 7bulls.com"},"change_message_id":"b7240db974fccc1c9f396b658baddf0caf7aaaf6","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                        installs \u003d [\"install\", \"uninstall\"]"},{"line_number":149,"context_line":"            else:"},{"line_number":150,"context_line":"                # Remove any uninstallations if we are trying to install"},{"line_number":151,"context_line":"                # the package without uninstallation elsewhere."},{"line_number":152,"context_line":"                data[phase][\"uninstall\"] \u003d ["},{"line_number":153,"context_line":"                    x for x in data[phase][\"uninstall\"] if x[0] !\u003d pkg_name]"},{"line_number":154,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"9f560f44_0ceaac0d","line":151,"range":{"start_line":151,"start_character":53,"end_line":151,"end_character":62},"in_reply_to":"9f560f44_b1a5d949","updated":"2020-08-21 16:21:20.000000000","message":"Makes sense, thank you. I missed the lack of control over ordering.","commit_id":"788224cfe074a55dfe4ffcf5f77f05bd5c32a8b5"}]}