Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006270OCamlOCaml tools (ocaml{lex,yacc,dep,browser,debug})public2013-12-13 23:462014-03-31 13:51
Reporterjwatzmanfb 
Assigned Toxclerc 
PrioritynormalSeverityminorReproducibilityN/A
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version 
Summary0006270: [PATCH] Remove need for -I directives to ocamldebug in common case
DescriptionThe need for a long list of -I directives makes interactively using ocamldebug a pain in the butt. Many folks have solved this with various `find` invocations or even Python wrappers, but those lead to other problems when it might include files you weren't expecting (or miss things you were). But all of this is really annoying since the tooling should be able to figure out itself, even heuristically, where your source files are -- gdb gets this right, why can't we?

This patch implements one of the more important heuristics from gdb: you typically debug on the same machine you built on, so looking for the source files and built artifacts in the absolute paths where they were during compilation is a good first try. We write out absolute paths into the debug events, and then automatically append the dirname from each such path into the load path. We also update source_of_module to use an absolute path if passed one, falling back on looking for the file's basename in the usual directory search if that absolute path isn't found. (Ideally these two mechanisms would be the same, but that's a problem for another day. :))

The load path appending behavior combined with the absolute path basename fallback mean that if you happen to be debugging on a machine where the original source and build artifacts are *not* available in their original absolute locations, things will work as before, using the standard load path mechanism. You can also explicitly use -I to prepend directories to the load path and override the defaults located by this new mechanism.

I personally find this makes using ocamldebug much more pleasant :)

(PS: This is my second time filing a Mantis ticket and sending a patch for ocaml, and my first hasn't been responded to yet (0006267). I wasn't able to find any documentation on your preferred format for receiving patches, so I hope this is acceptable. If you prefer something else, please let me know and I'd be happy to oblige!)
Tagspatch
Attached Filespatch file icon ocamldebug-filename.patch [^] (3,827 bytes) 2013-12-13 23:46 [Show Content]

- Relationships

-  Notes
(0010794)
yallop (developer)
2014-01-13 11:22

The patch works as advertised for me, and seems like a significant usability improvement.
(0010839)
frisch (developer)
2014-01-23 19:19

(Disclaimer: comments below are only based on a quick read of the patch. I haven't tried it.)

Always turning filename into absolute paths in debug event locations can drawbacks:

 - Location displayed in backtraces shows filenames meaningful only on the development machine.

 - The size of the generated bytecode or .cds file could grow significantly, esp. because sharing is lost between all the pos_fname strings (always re-created from absolute_path). This could be mitigated by memoizing the absolute_path function.
(0010840)
jwatzmanfb (reporter)
2014-01-23 19:36

> - Location displayed in backtraces shows filenames meaningful only on the development machine.

Ah, yeah, good point. Do you think it would be better to add a new field next to ev_loc, like ev_abs_path, that is used only for this purpose? I think this would work well for add_dirs_from_symbols but source_of_module would need some plumbing (which I'm willing to do, just checking first :)).

> - The size of the generated bytecode or .cds file could grow significantly, esp. because sharing is lost between all the pos_fname strings (always re-created from absolute_path). This could be mitigated by memoizing the absolute_path function.

Didn't realize the details of this, thanks for letting me know. I'll make sure this is fixed when I fix the above issue.
(0010841)
xclerc (developer)
2014-01-24 09:18

I basically agree with Alain, and am wondering:
would it be acceptable to emit absolute paths in
debug events iff the "-absname" switch is passed
to the compiler? Of course, even if use of absolute
paths is triggered by a switch, sharing paths
between events is still useful.
(0010842)
frisch (developer)
2014-01-24 09:50

> I basically agree with Alain, and am wondering:
> would it be acceptable to emit absolute paths in
> debug events iff the "-absname" switch is passed
> to the compiler?

We use -absname for all our code (to facilitate jumping to the location of errors), but would not like to increase the size of bytecode/.cds file nor to include full paths in backtrace. (Internally we use bytecode only for internal development -- so we would not really mind about backtraces containing full paths. But some of our clients might recompile and use bytecode in production on platforms which don't have a native backend.)
(0010843)
frisch (developer)
2014-01-24 09:51

> Do you think it would be better to add a new field next to ev_loc, like ev_abs_path, that is used only for this purpose?

Do we really want to include absolute path information directly in debug events? What about using an extra (optional) section in the .cds or bytecode files?
(0010845)
jwatzmanfb (reporter)
2014-01-25 01:18

> Do we really want to include absolute path information directly in debug events?

No, we probably don't need to, it was just the only way that I knew how to do it :)

> What about using an extra (optional) section in the .cds or bytecode files?

That sounds like a better idea, I will look into this. Do you have a pointer to where the sections to the cds and bytecode files are defined/serialized, or documentation about it? If not, no worries, I'm sure I can dig in and figure it out eventually.
(0010936)
jwatzmanfb (reporter)
2014-02-18 04:10

Given that you are now accepting pull requests on github, I just sent https://github.com/ocaml/ocaml/pull/14 [^] as an update to this ticket.

Sorry to take so log to respond -- this is a side project for me and I just found time to work on it again earlier today :)
(0011086)
jwatzmanfb (reporter)
2014-03-23 20:25

Ping again :) I'm told that one of xclerc or frisch needs to sign off on the patch getting incorporated into SVN -- would one of you mind to take a look and either commit the patch or provide feedback that I can use to make another revision?

The previous version has been sitting for about a month, and I just made another minor tweak to it which makes it even more non-intrusive. Would appreciate the review and am happy to keep revising it if it's still not acceptable, just let me know here or on GitHub.

Thanks!
(0011138)
xclerc (developer)
2014-03-31 13:51

The patch, as reviewed/commented/amended on github,
has been committed in trunk (revision 14507).

- Issue History
Date Modified Username Field Change
2013-12-13 23:46 jwatzmanfb New Issue
2013-12-13 23:46 jwatzmanfb File Added: ocamldebug-filename.patch
2014-01-13 11:08 yallop Tag Attached: patch
2014-01-13 11:22 yallop Note Added: 0010794
2014-01-23 19:19 frisch Note Added: 0010839
2014-01-23 19:36 jwatzmanfb Note Added: 0010840
2014-01-24 09:18 xclerc Note Added: 0010841
2014-01-24 09:50 frisch Note Added: 0010842
2014-01-24 09:51 frisch Note Added: 0010843
2014-01-25 01:18 jwatzmanfb Note Added: 0010845
2014-02-18 04:10 jwatzmanfb Note Added: 0010936
2014-03-23 20:25 jwatzmanfb Note Added: 0011086
2014-03-31 13:51 xclerc Note Added: 0011138
2014-03-31 13:51 xclerc Status new => resolved
2014-03-31 13:51 xclerc Resolution open => fixed
2014-03-31 13:51 xclerc Assigned To => xclerc


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker