Skip to content
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

doc: Update explanation of configure #1763

Closed
wants to merge 0 commits into from

Conversation

SEOKMIN83
Copy link
Contributor

@SEOKMIN83 SEOKMIN83 commented Jul 17, 2023

Add the part about the libunwind package that being missed from the slide and an install command if you want to use the that package.

Rather than subtracting the libunwind package portion of I think it would be less confusing for users if we described it correctly.

Copy link
Collaborator

@honggyukim honggyukim left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, but this change was already discussed and decided not to have it.

doc/uftrace.html Outdated
@@ -247,6 +247,7 @@
... perf_event: [ .green[on] ] - perf (PMU) event support
... schedule: [ .green[on] ] - scheduler event support
... capstone: [ .red[OFF] ] - full dynamic tracing support
... libunwind: [ .red[OFF] ] - stacktrace support support (optional for debugging)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not required here. Please read the previous discussion at #1744 (comment).

doc/uftrace.html Outdated
... libunwind: [ .red[OFF] ] - stacktrace support support (optional for debugging)

# If you want to turn on libunwind option
$ sudo apt-get install -y libunwind-dev
Copy link
Collaborator

Choose a reason for hiding this comment

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

This tutorial slide doesn't have to cover all the details so no need to have this change.

@SEOKMIN83 SEOKMIN83 changed the title Update explanation of configure. doc: Update explanation of configure Jul 30, 2023
@namhyung
Copy link
Owner

namhyung commented Aug 4, 2023

As @honggyukim said, we intentionally dropped it. Please add a comment instead to prevent confusion later.

@paranlee
Copy link
Contributor

If libunwind-N package have dashed number, then configure can't find libunwind.
So, configure didn't find libunwind-N.

libunwind-15/unstable,now 1:15~++20220509105605+460fc79a080b-1~exp1 riscv64 [installed,automatic]
  production-quality unwinder

libunwind-15-dbgsym/unstable 1:15~++20220509105605+460fc79a080b-1~exp1 riscv64
  debug symbols for libunwind-15

libunwind-15-dev/unstable,now 1:15~++20220509105605+460fc79a080b-1~exp1 riscv64 [installed]
  production-quality unwinder

@SEOKMIN83
Copy link
Contributor Author

As @honggyukim said, we intentionally dropped it. Please add a comment instead to prevent confusion later.

Then can I put a comment on the slide like below?
"libunwind is optional. so we skipped this part. in tutorial."

@SEOKMIN83 SEOKMIN83 closed this Aug 20, 2023
@SEOKMIN83 SEOKMIN83 reopened this Aug 20, 2023
@SEOKMIN83
Copy link
Contributor Author

If libunwind-N package have dashed number, then configure can't find libunwind. So, configure didn't find libunwind-N.

libunwind-15/unstable,now 1:15~++20220509105605+460fc79a080b-1~exp1 riscv64 [installed,automatic]
  production-quality unwinder

libunwind-15-dbgsym/unstable 1:15~++20220509105605+460fc79a080b-1~exp1 riscv64
  debug symbols for libunwind-15

libunwind-15-dev/unstable,now 1:15~++20220509105605+460fc79a080b-1~exp1 riscv64 [installed]
  production-quality unwinder

It would be nice if you could explain in a little more detail that it is a problem that occurs in RISC-V.
And it would be better to subtract this part from the slide, right?

@honggyukim
Copy link
Collaborator

As @honggyukim said, we intentionally dropped it. Please add a comment instead to prevent confusion later.

Then can I put a comment on the slide like below? "libunwind is optional. so we skipped this part. in tutorial."

I think it's too verbose adding a comment inside markdown slide because it hurts the readability. So I'm sorry but it's better to be dropped.

@honggyukim
Copy link
Collaborator

If libunwind-N package have dashed number, then configure can't find libunwind.
So, configure didn't find libunwind-N.

@paranlee Thanks for the report. But I think it's not the one we have to handle.

@honggyukim
Copy link
Collaborator

Besides that, I actually think about adding libunwind-dev by default in both release and debug build. But it could be a future work item so we can recall this PR when it's really needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants