Made package index support plaintext #80
Conversation
antholeole
force-pushed
the
oleina/plaintext
branch
3 times, most recently
from
aa86670
to
d5f00f6
Compare
antholeole
force-pushed
the
oleina/plaintext
branch
from
d5f00f6
to
9e0f7bb
Compare
|
@antholeole nice! IMHO the more support for all indexes, the better :) Also, personally, I'd add tests to the PR so that there won't be regressions in the future 😃 |
antholeole
force-pushed
the
oleina/plaintext
branch
from
9e0f7bb
to
e13ea5f
Compare
|
@jjmaestro added tests! ptal again |
apt/private/package_index.bzl
Outdated
|
|
||
| failure = None | ||
| if not download.success: | ||
| failure = (url, download, None) | ||
| else: | ||
| # there is a decompression step; we shouldn't consider this a success | ||
| # until we successfully decompress. | ||
| if len(cmd) > 0: | ||
| decompress_r = rctx.execute(cmd + [output]) | ||
| if decompress_r.return_code == 0: | ||
| integrity = download.integrity | ||
| else: | ||
| failure = (dist_url, download, decompress_r) | ||
|
|
||
| if failure == None: | ||
| break | ||
| else: | ||
| failed_attempts.append(failure) |
There was a problem hiding this comment.
IMHO you don't need to change all of these lines / logic, you can simply leave all of the previous code and add this check in L31 before the decompression:
if download.success and not ext:
break
|
I've re-read the commit and I don't think the test is exercising the code. The extensions in Also, the test is IMHO a bit messy since it's adding the "exception" for Bazel on So... if there's no single debian repo with only a plain text index, it's going to be hard to add a test that exercises the code. Sorry for my initial suggestion! 😅 I still think it's a valuable addition to support plaintext indexes since it's a valid index format per the Debian repository format spec so hopefully it'll be merged! |
This is useful for something like https://storage.googleapis.com/bazel-apt/, which only supports plaintext. Signed-off-by: oleina <[email protected]> Change-Id: Ic3698c5e281b24a6017c034c164b657d1076a11e
|
Thats so strange. When i was writing the PR yesterday, those .gz packages did not exist despite their LastModified claiming it did... I'll leave the test up, since despite not actually testing no-extension indexes - it also tests a variety of other intestesting things, like using packages from two different repos as well as non-debian.org indexes. regardless, applied your suggestion! |
antholeole
force-pushed
the
oleina/plaintext
branch
from
e13ea5f
to
3a8ed52
Compare
|
@antholeole I still think it's not great to change the behavior of the e2e test depending on the architecture, especially when the test is not really exercising anything in the new code or anything special in the old code.
Unless I'm mistaken, that's is already the default. Every
Well, either it follows the Debian specs or it will fail, regardless of the URL being a debian.org URL so... 🤷 If you really want to have something like this, you could create a new example in the In any case, I'm not a maintainer of the repo, so this is just my opinion! |
This is useful for something like https://storage.googleapis.com/bazel-apt/, which (edit: used to!) only supports plaintext.
Change-Id: Ic3698c5e281b24a6017c034c164b657d1076a11e