Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 2:20 PM Mike Gilbert wrote: > > On Mon, Jul 11, 2022 at 2:01 PM Anna wrote: > > > > On 2022-07-11 19:57, Ulrich Mueller wrote: > > > > On Mon, 11 Jul 2022, Mike Gilbert wrote: > > > > > > >> Maybe leave ebegin/eend in place then, which was invented precisely for > > > >> this use case? What's so bad about nesting it? > > > > > > > It leads to odd looking output. > > > > > > > https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7 > > > > > > IIUC it would look like this, with the patch applied: > > > > > > * task 1 > > > * Doing task 2 ... [ ok ] > > > * task 1 succeeded > > > > > > That's not the most beautiful of outputs either. > > > > I think ebegin/eend output should be buffered so it can be nested > > properly. > > > > My take: the main purpose of ebegin is to inform the user that we are > starting a silent long-running task. eend tells you the result of that > task. > > Buffering ebegin calls would sort of defeat that purpose of informing > the user that something is happening. > > In other words, I don't think it makes sense to call ebegin before > starting a task that is expected to produce output. This includes > tasks that call ebegin themselves, since ebegin always produces > output. I've decided this is a hill I don't want to die on, so I've submitted a PR to update the QA check. https://github.com/gentoo/portage/pull/854
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 2:49 PM Frederik Pfautsch wrote: > > Am 11.07.22 um 20:14 schrieb Mike Gilbert: > > On Mon, Jul 11, 2022 at 1:57 PM Ulrich Mueller wrote: > >> > >>> On Mon, 11 Jul 2022, Mike Gilbert wrote: > >> > Maybe leave ebegin/eend in place then, which was invented precisely for > this use case? What's so bad about nesting it? > >> > >>> It leads to odd looking output. > >> > >>> https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7 > >> > >> IIUC it would look like this, with the patch applied: > >> > >> * task 1 > >> * Doing task 2 ... [ ok ] > >> * task 1 succeeded > >> > >> That's not the most beautiful of outputs either. > > > > Right. I would prefer to apply the first patch I submitted instead. > > How about output like: > > * Doing task 1 ... > > | * Doing task 2 ... > > | \> [ ok ] > > | > > | additional log from task 1 > > | line 2 of task 1 log > > \> [ !! ] > > This would require keeping track of the current "indentation" > level/prepend "| " to each output for every level. But not sure if is > possible/how difficult it is to implement. Just sort of a quick idea. That seems like it would be somewhat involved to implement, and it seems like a lot of complexity to solve relatively minor problem.
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
Am 11.07.22 um 20:14 schrieb Mike Gilbert: On Mon, Jul 11, 2022 at 1:57 PM Ulrich Mueller wrote: On Mon, 11 Jul 2022, Mike Gilbert wrote: Maybe leave ebegin/eend in place then, which was invented precisely for this use case? What's so bad about nesting it? It leads to odd looking output. https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7 IIUC it would look like this, with the patch applied: * task 1 * Doing task 2 ... [ ok ] * task 1 succeeded That's not the most beautiful of outputs either. Right. I would prefer to apply the first patch I submitted instead. How about output like: * Doing task 1 ... | * Doing task 2 ... | \> [ ok ] | | additional log from task 1 | line 2 of task 1 log \> [ !! ] This would require keeping track of the current "indentation" level/prepend "| " to each output for every level. But not sure if is possible/how difficult it is to implement. Just sort of a quick idea. OpenPGP_signature Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 2:01 PM Anna wrote: > > On 2022-07-11 19:57, Ulrich Mueller wrote: > > > On Mon, 11 Jul 2022, Mike Gilbert wrote: > > > > >> Maybe leave ebegin/eend in place then, which was invented precisely for > > >> this use case? What's so bad about nesting it? > > > > > It leads to odd looking output. > > > > > https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7 > > > > IIUC it would look like this, with the patch applied: > > > > * task 1 > > * Doing task 2 ... [ ok ] > > * task 1 succeeded > > > > That's not the most beautiful of outputs either. > > I think ebegin/eend output should be buffered so it can be nested > properly. > My take: the main purpose of ebegin is to inform the user that we are starting a silent long-running task. eend tells you the result of that task. Buffering ebegin calls would sort of defeat that purpose of informing the user that something is happening. In other words, I don't think it makes sense to call ebegin before starting a task that is expected to produce output. This includes tasks that call ebegin themselves, since ebegin always produces output.
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 1:57 PM Ulrich Mueller wrote: > > > On Mon, 11 Jul 2022, Mike Gilbert wrote: > > >> Maybe leave ebegin/eend in place then, which was invented precisely for > >> this use case? What's so bad about nesting it? > > > It leads to odd looking output. > > > https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7 > > IIUC it would look like this, with the patch applied: > > * task 1 > * Doing task 2 ... [ ok ] > * task 1 succeeded > > That's not the most beautiful of outputs either. Right. I would prefer to apply the first patch I submitted instead.
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
> On Mon, 11 Jul 2022, Mike Gilbert wrote: >> Maybe leave ebegin/eend in place then, which was invented precisely for >> this use case? What's so bad about nesting it? > It leads to odd looking output. > https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7 IIUC it would look like this, with the patch applied: * task 1 * Doing task 2 ... [ ok ] * task 1 succeeded That's not the most beautiful of outputs either. signature.asc Description: PGP signature
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 1:17 PM Ionen Wolkens wrote:
>
> On Mon, Jul 11, 2022 at 01:08:52PM -0400, Mike Gilbert wrote:
> > On Mon, Jul 11, 2022 at 12:56 PM Ulrich Mueller wrote:
> > >
> > > > On Mon, 11 Jul 2022, Ionen Wolkens wrote:
> > > >> -ebegin " python_check_deps"
> > > >> -python_check_deps
> > > >> -eend ${?}
> > > >> +einfo " python_check_deps"
> > > >> +if python_check_deps; then
> > > >> +einfo " python_check_deps succeeded"
> > > >> +else
> > > >> +einfo " python_check_deps failed"
> > > >> +fi
> > > >> }
> > >
> > > > I was about to go about merging this as suggested, but this masks the
> > > > return value, and then things like this always succeed:
> > >
> > > > if _python_run_check_deps "${impl}"; then
> > >
> > > Maybe leave ebegin/eend in place then, which was invented precisely for
> > > this use case? What's so bad about nesting it?
> >
> > It leads to odd looking output.
> >
> > https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7
> >
>
> Isn't that a portage problem? I don't think stopping legitimate use
> because portage displays messages weird is the right thing to do but
> should rather improve how portage displays them in this situation.
What is a "good" way to display it? I don't really think you can make
ebegin/eend look good when there are lines of output between them.
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 01:08:52PM -0400, Mike Gilbert wrote:
> On Mon, Jul 11, 2022 at 12:56 PM Ulrich Mueller wrote:
> >
> > > On Mon, 11 Jul 2022, Ionen Wolkens wrote:
> > >> -ebegin " python_check_deps"
> > >> -python_check_deps
> > >> -eend ${?}
> > >> +einfo " python_check_deps"
> > >> +if python_check_deps; then
> > >> +einfo " python_check_deps succeeded"
> > >> +else
> > >> +einfo " python_check_deps failed"
> > >> +fi
> > >> }
> >
> > > I was about to go about merging this as suggested, but this masks the
> > > return value, and then things like this always succeed:
> >
> > > if _python_run_check_deps "${impl}"; then
> >
> > Maybe leave ebegin/eend in place then, which was invented precisely for
> > this use case? What's so bad about nesting it?
>
> It leads to odd looking output.
>
> https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7
>
Isn't that a portage problem? I don't think stopping legitimate use
because portage displays messages weird is the right thing to do but
should rather improve how portage displays them in this situation.
--
ionen
signature.asc
Description: PGP signature
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 12:11 PM Ionen Wolkens wrote:
>
> On Mon, Jul 11, 2022 at 09:16:10AM -0400, Mike Gilbert wrote:
> > It's common for python_check_deps to call python_has_version, which
> > calls ebegin itself.
> >
> > Signed-off-by: Mike Gilbert
> > ---
> > eclass/python-utils-r1.eclass | 9 ++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > index a18ca58475f..5c678c524ae 100644
> > --- a/eclass/python-utils-r1.eclass
> > +++ b/eclass/python-utils-r1.eclass
> > @@ -1399,9 +1399,12 @@ _python_run_check_deps() {
> >
> > local PYTHON_USEDEP="python_targets_${impl}(-)"
> > local PYTHON_SINGLE_USEDEP="python_single_target_${impl}(-)"
> > - ebegin " python_check_deps"
> > - python_check_deps
> > - eend ${?}
> > + einfo " python_check_deps"
> > + if python_check_deps; then
> > + einfo " python_check_deps succeeded"
> > + else
> > + einfo " python_check_deps failed"
> > + fi
> > }
>
> I was about to go about merging this as suggested, but this masks the
> return value, and then things like this always succeed:
>
> if _python_run_check_deps "${impl}"; then
Thanks for catching that.
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 12:56 PM Ulrich Mueller wrote:
>
> > On Mon, 11 Jul 2022, Ionen Wolkens wrote:
> >> -ebegin " python_check_deps"
> >> -python_check_deps
> >> -eend ${?}
> >> +einfo " python_check_deps"
> >> +if python_check_deps; then
> >> +einfo " python_check_deps succeeded"
> >> +else
> >> +einfo " python_check_deps failed"
> >> +fi
> >> }
>
> > I was about to go about merging this as suggested, but this masks the
> > return value, and then things like this always succeed:
>
> > if _python_run_check_deps "${impl}"; then
>
> Maybe leave ebegin/eend in place then, which was invented precisely for
> this use case? What's so bad about nesting it?
It leads to odd looking output.
https://gitweb.gentoo.org/proj/portage.git/commit/?id=eba088af8f335c0adb386461e6df1267e24800e7
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
> On Mon, 11 Jul 2022, Ionen Wolkens wrote:
>> -ebegin " python_check_deps"
>> -python_check_deps
>> -eend ${?}
>> +einfo " python_check_deps"
>> +if python_check_deps; then
>> +einfo " python_check_deps succeeded"
>> +else
>> +einfo " python_check_deps failed"
>> +fi
>> }
> I was about to go about merging this as suggested, but this masks the
> return value, and then things like this always succeed:
> if _python_run_check_deps "${impl}"; then
Maybe leave ebegin/eend in place then, which was invented precisely for
this use case? What's so bad about nesting it?
Ulrich
signature.asc
Description: PGP signature
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, Jul 11, 2022 at 09:16:10AM -0400, Mike Gilbert wrote:
> It's common for python_check_deps to call python_has_version, which
> calls ebegin itself.
>
> Signed-off-by: Mike Gilbert
> ---
> eclass/python-utils-r1.eclass | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> index a18ca58475f..5c678c524ae 100644
> --- a/eclass/python-utils-r1.eclass
> +++ b/eclass/python-utils-r1.eclass
> @@ -1399,9 +1399,12 @@ _python_run_check_deps() {
>
> local PYTHON_USEDEP="python_targets_${impl}(-)"
> local PYTHON_SINGLE_USEDEP="python_single_target_${impl}(-)"
> - ebegin " python_check_deps"
> - python_check_deps
> - eend ${?}
> + einfo " python_check_deps"
> + if python_check_deps; then
> + einfo " python_check_deps succeeded"
> + else
> + einfo " python_check_deps failed"
> + fi
> }
I was about to go about merging this as suggested, but this masks the
return value, and then things like this always succeed:
if _python_run_check_deps "${impl}"; then
>
> # @FUNCTION: python_has_version
> --
> 2.37.0
>
>
--
ionen
signature.asc
Description: PGP signature
Re: [gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
On Mon, 2022-07-11 at 09:16 -0400, Mike Gilbert wrote:
> It's common for python_check_deps to call python_has_version, which
> calls ebegin itself.
>
> Signed-off-by: Mike Gilbert
> ---
> eclass/python-utils-r1.eclass | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> index a18ca58475f..5c678c524ae 100644
> --- a/eclass/python-utils-r1.eclass
> +++ b/eclass/python-utils-r1.eclass
> @@ -1399,9 +1399,12 @@ _python_run_check_deps() {
>
> local PYTHON_USEDEP="python_targets_${impl}(-)"
> local PYTHON_SINGLE_USEDEP="python_single_target_${impl}(-)"
> - ebegin " python_check_deps"
> - python_check_deps
> - eend ${?}
> + einfo " python_check_deps"
> + if python_check_deps; then
> + einfo " python_check_deps succeeded"
> + else
> + einfo " python_check_deps failed"
> + fi
> }
>
> # @FUNCTION: python_has_version
WFM. Please don't push it yet, I'll ask ionen to add it on top of his
patchset for a single merge.
--
Best regards,
Michał Górny
[gentoo-dev] [PATCH v2] python-utils-r1.eclass: avoid nested ebegin calls
It's common for python_check_deps to call python_has_version, which
calls ebegin itself.
Signed-off-by: Mike Gilbert
---
eclass/python-utils-r1.eclass | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index a18ca58475f..5c678c524ae 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1399,9 +1399,12 @@ _python_run_check_deps() {
local PYTHON_USEDEP="python_targets_${impl}(-)"
local PYTHON_SINGLE_USEDEP="python_single_target_${impl}(-)"
- ebegin " python_check_deps"
- python_check_deps
- eend ${?}
+ einfo " python_check_deps"
+ if python_check_deps; then
+ einfo " python_check_deps succeeded"
+ else
+ einfo " python_check_deps failed"
+ fi
}
# @FUNCTION: python_has_version
--
2.37.0
