Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Make building with debuginfo optional #33

Merged
merged 1 commit into from
Mar 28, 2018

Conversation

major
Copy link
Contributor

@major major commented Mar 27, 2018

By default, kernels are built with debug symbols. This increases the
build time and the size of the resulting tarball.

tar.gz with debug symbols: ~ 250MB
tar.gz without debug symbols: ~ 75MB

If debug symbols are really needed for diagnosing a tough problem,
the user can specify --enable-debuginfo when calling skt build.

Fixes #32.

Signed-off-by: Major Hayden major@redhat.com

@stefwalter
Copy link
Contributor

Needs a rebase.

@major major force-pushed the issue-14-disable-debuginfo branch from 5731788 to 44879b3 Compare March 27, 2018 21:36
@major
Copy link
Contributor Author

major commented Mar 27, 2018

@stefwalter Done!

https://media.giphy.com/media/8VrtCswiLDNnO/giphy.gif

@stefwalter
Copy link
Contributor

Just a cross check on the default here.

So I'm interested in the "why" ... do you expect building a kernel without debuginfo to be routine and normal case for SKT runs? If not, why is it the default?

Does a non-debuginfo kernel significantly increase the difficulty of diagnosing a test failure? @dzickusrh @veruu Any comments?

skt/__init__.py Outdated
if not self.enable_debuginfo:
logging.info("disabling debuginfo:")
args = shlex.split('script/config --disable debug_info')
subprocess.check_call(args)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably just be:

subprocess.check_call(['script/config', '--disable', 'debug_info'])

And drop the shlex usage? What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed, but shlex is usually my go-to for increasing readability. ;)

I'll change it.

@veruu
Copy link
Contributor

veruu commented Mar 28, 2018

Does a non-debuginfo kernel significantly increase the difficulty of diagnosing a test failure? @dzickusrh @veruu Any comments?

Based on my experience, debug info is only useful if you want to run the resulting image through gdb / objdump etc but it doesn't add anything otherwise. Will we keep kernel images that failed testing somewhere for future debugging? If not, we don't need debug info, IMHO (unless there is another use case that I'm currently overlooking).

@stefwalter
Copy link
Contributor

Sounds like defaulting to without debuginfo makes sense.

@major major force-pushed the issue-14-disable-debuginfo branch from 44879b3 to 9974d65 Compare March 28, 2018 13:17
@major
Copy link
Contributor Author

major commented Mar 28, 2018

Okay, the patch is updated. I also added a quick note in the README.md about the change.

@dzickusrh
Copy link
Contributor

This comment is a little late, but just to add to it. I agree with Veronika, for the most part debuginfo isn't useful. The only piece in there that folks use is 'vmlinux' to feed into crash. Most local builds we strip out the debug symbols to massively reduce the size.
I usually accomplish that with a for-loop and a 'strip -g' on all kernel modules. I didn't realize a simple DEBUG_INFO option could be disabled. Learn something new everyday..

Copy link
Contributor

@spbnick spbnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, need just one letter changed :)

# final kernel tarball size by 3-4x and can increase build time
# slightly. Debug symbols are really only needed for deep diagnosis
# of kernel issues on a specific system. This is why debuginfo is
# disabled by default.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, nice touch with the comment :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

cfg.get('cfgtype'),
cfg.get('makeopts'),
cfg.get('enable_debuginfo')
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting a little out of hand. We'll need to convert this to named arguments next.

Copy link
Contributor Author

@major major Mar 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I can work on that. Created #42 for that.

"--enable-debuginfo",
type=bool,
default=False,
help="build kernel with debuginfo (default: disabled)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please start the help text with a capital letter to match the rest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

By default, kernels are built with debug symbols. This increases the
build time and the size of the resulting tarball.

  tar.gz with debug symbols: ~ 250MB
  tar.gz without debug symbols: ~ 75MB

If debug symbols are really needed for diagnosing a tough problem,
the user can specify `--enable-debuginfo` when calling `skt build`.

Closes cki-project#32.
@major major force-pushed the issue-14-disable-debuginfo branch from 9974d65 to 702a976 Compare March 28, 2018 16:36
@spbnick spbnick merged commit 0d3c31f into cki-project:master Mar 28, 2018
@spbnick
Copy link
Contributor

spbnick commented Mar 28, 2018

Thank you, Major :)!

@major major deleted the issue-14-disable-debuginfo branch March 28, 2018 16:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants