Re: [PATCH 1/1] Github Actions: cache util-linux tarball
Namjae Jeon <[email protected]>
| Newsgroups | dev.linux.lists.exfat |
|---|---|
| Message-ID | <CAKYAXd-TyX=wx7mHVjr9uOL6s964dXDc=jLtRTTr-PC_v7SRYQ@mail.gmail.com> |
> >> @@ -188,12 +189,29 @@ jobs:
> >> sudo apt-get install -y linux-modules-extra-$(uname -r)
> >> export LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH
> >> export PATH=/usr/local/lib:$PATH
> >> +
> >> + - name: Retrieve util-linux tarball from cache
> >> + uses: actions/cache/restore@v6
> >> + id: restore-util-linux-tarball
> >> + with:
> >> + path: ${{ env.UTIL_LINUX_TARBALL }}
> >> + key: tarball-${{ env.UTIL_LINUX_TARBALL }}
> >> + - name: Download util-linux tarball
> >> + if: steps.restore-util-linux-tarball.outputs.cache-hit != 'true'
> >> + run: curl -L -o "$UTIL_LINUX_TARBALL" "$UTIL_LINUX_SRC"
> >> + - name: Save util-linux tarball from cache
> >> + uses: actions/cache/save@v6
> >> + if: steps.restore-util-linux-tarball.outputs.cache-hit != 'true'
> >> + with:
> >> + path: ${{ env.UTIL_LINUX_TARBALL }}
> >> + key: tarball-${{ env.UTIL_LINUX_TARBALL }}
> >> +
> First off, this is AI report? If so, could you please come clean and say
> that the review is AI-assisted? I'm starting to get tired of all the
> pedanticity the LLM is throwing at me.
I am merely asking a question about a patch I was trying to thoroughly
review. Are your questions really not worth answering? I could be
mistaken, and you can refute my points. That is, after all, what the
mailing list is for.
Recently, the sheer volume of AI-generated patches has been taking
over my life. So please don't complain to me about getting tired over
something like this.
I entertained the thought for a split second that some of the patches
might be AI-generated, but I never questioned about it. because doing
so would be very disrespectful if it is not true.
>
> > There is one ordering problem. The cache and download steps run before
> > actions/checkout. Since the tarball is stored in the workspace,
> > actions/checkout may clean the workspace and delete it. The later tar
> > xf command will then fail because the tarball is gone. I am wondering
> > if this patch works as intended. Should we move the checkout step
> > before the cache steps?
> No, there isn't. This is just wrong. libblkid is untared and installed
> to the runner system before checking out. This report is invalid. I
> attached the links in the cover letter to prove that I tested this
> patch(if the reviewer model had access to the links and actually visited
> them, I think Claude is smart enough to realise its wrong take). The
> runs didn't fail and the first run cached the tarball and the second run
> hit the cache and the curl step is skipped.
>
> >
> > Also, please add --fail to curl:
> >
> > curl --fail --location --retry 3 \
> > -o "$UTIL_LINUX_TARBALL" "$UTIL_LINUX_SRC"
> >
> > Without --fail, an HTTP error such as 404 or 500 may still produce a
> > successful curl exit status, and the error response could be saved and
> > cached as the tarball.
> >
> No. This doesn't really fix any real issue. Once the tarball is cached,
> the step will probably never be called ever again for the
> org(exfatprogs), unless it's forked elsewhere or more than 10GB of cache
> is used in which case the tarball got evicted.
Okay.
>
> If the CDN is in a such condition that it returns 5xx errors,
> immediately retrying the request wouldn't make any difference. The
> failure mode we were experiencing was at the L4 level(TCP connection
> dropping out or SYN being dropped), probably either due to the WAF at
> the CDN end or the Azure VM network being saturated. There is not much
> we can do for the first cache miss run.
Okay. I will apply this patch.
Thanks.