New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qubes-linux-template-builder verbosity disabling bug #1100

Closed
adrelanos opened this Issue Aug 4, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@adrelanos
Member

adrelanos commented Aug 4, 2015

There is a problem with setVerboseMode.

When xtrace is already set, then setVerboseMode actually disabled xtrace. (Because it's not cached in variable XTRACE.) This could be reproduced by sourceing functions.sh while set -x is already set.

setVerboseMode is only used in two places:

getXtrace and setXtrace is just used once in function umount_kill. Would it really be that verbose to justify going in circles just for temporarily disabling xtrace?

Can we remove whole setVerboseMode, getXtrace and setXtrace? I could provide a pull request for it. Or can you fix it?

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Aug 4, 2015

Member

Feel free to remove setVerboseMode, IMO we have better things to do than thinking
how to fix it. Regarding getXtrace and setXtrace in umount_kill - I
think it's a good idea to keep them (maybe just an inline if you want) -
umount_kill is called also after failed build and is pretty verbose -
will make much harder to find the actual error message.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Member

marmarek commented Aug 4, 2015

Feel free to remove setVerboseMode, IMO we have better things to do than thinking
how to fix it. Regarding getXtrace and setXtrace in umount_kill - I
think it's a good idea to keep them (maybe just an inline if you want) -
umount_kill is called also after failed build and is pretty verbose -
will make much harder to find the actual error message.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

adrelanos added a commit to adrelanos/qubes-builder-debian that referenced this issue Aug 4, 2015

remove call of function setVerboseMode and variable XTRACE
fix `qubes-linux-template-builder verbosity disabling bug`
QubesOS/qubes-issues#1100

@adrelanos adrelanos referenced this issue in marmarek/qubes-builder-debian Aug 4, 2015

Merged

remove call of function setVerboseMode and variable XTRACE #15

adrelanos added a commit to adrelanos/qubes-builder-fedora that referenced this issue Aug 4, 2015

remove call of function setVerboseMode and variable XTRACE
fix `qubes-linux-template-builder verbosity disabling bug`
QubesOS/qubes-issues#1100

adrelanos added a commit to adrelanos/qubes-linux-template-builder that referenced this issue Aug 4, 2015

fixed 'verbosity disabling bug'
QubesOS/qubes-issues#1100
Deprecated functions setVerboseMode, getXtrace, setXtrace and variable XTRACE, because those were broken and their only use case was function umount_kill. Re-implemented disabling xtrace in function umount_kill, if variable VERBOSE is lower than or equal 2.

@adrelanos adrelanos referenced this issue in marmarek/old-qubes-linux-template-builder Aug 4, 2015

Merged

fixed 'verbosity disabling bug' #8

@adrelanos

This comment has been minimized.

Show comment
Hide comment
@adrelanos

adrelanos Aug 4, 2015

Member

Pull requests are attached above.

Member

adrelanos commented Aug 4, 2015

Pull requests are attached above.

@adrelanos

This comment has been minimized.

Show comment
Hide comment
@adrelanos

adrelanos Aug 4, 2015

Member

adrelanos/qubes-linux-template-builder@e84645e#commitcomment-12529288

Actual disabling xtrace got lost here...

Indeed. If it's otherwise mergeable, could you please merge it and add the missing set +x?

Member

adrelanos commented Aug 4, 2015

adrelanos/qubes-linux-template-builder@e84645e#commitcomment-12529288

Actual disabling xtrace got lost here...

Indeed. If it's otherwise mergeable, could you please merge it and add the missing set +x?

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Aug 4, 2015

Member

On Tue, Aug 04, 2015 at 04:24:09PM -0700, Patrick Schleizer wrote:

adrelanos/qubes-linux-template-builder@e84645e#commitcomment-12529288

Actual disabling xtrace got lost here...

Indeed. If it's otherwise mergeable, could you please merge it and add the missing set +x?

Sure.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Member

marmarek commented Aug 4, 2015

On Tue, Aug 04, 2015 at 04:24:09PM -0700, Patrick Schleizer wrote:

adrelanos/qubes-linux-template-builder@e84645e#commitcomment-12529288

Actual disabling xtrace got lost here...

Indeed. If it's otherwise mergeable, could you please merge it and add the missing set +x?

Sure.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

marmarek added a commit to marmarek/old-qubes-linux-template-builder that referenced this issue Aug 4, 2015

Merge remote-tracking branch 'origin/pr/8'
* origin/pr/8:
  fixed 'verbosity disabling bug' QubesOS/qubes-issues#1100 Deprecated functions setVerboseMode, getXtrace, setXtrace and variable XTRACE, because those were broken and their only use case was function umount_kill. Re-implemented disabling xtrace in function umount_kill, if variable VERBOSE is lower than or equal 2.
@adrelanos

This comment has been minimized.

Show comment
Hide comment
@adrelanos

adrelanos Aug 5, 2015

Member

Since my pull request was merged. And thanks to marek for adding the missing set +x:
marmarek/qubes-linux-template-builder@0b8d27c

Done.

Member

adrelanos commented Aug 5, 2015

Since my pull request was merged. And thanks to marek for adding the missing set +x:
marmarek/qubes-linux-template-builder@0b8d27c

Done.

@adrelanos adrelanos closed this Aug 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment