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.

Sign up for GitHub

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

Jump to bottom

[engine] Initial Trace implementation for rust engine #4076

Merged
merged 11 commits into from
Nov 19, 2016
Merged

[engine] Initial Trace implementation for rust engine #4076

merged 11 commits into from
Nov 19, 2016

Conversation

baroquebobcat
Copy link
Contributor

@baroquebobcat baroquebobcat commented Nov 18, 2016 •
edited
Loading

Problem

Re-implementing trace was deferred during the initial work on the Rust scheduler. See #4007.

Solution

This patch partially reintroduces it. The rust implementation will now produce traces that include the subjects and some of the information about failures. Some types of failures will not have good messages yet. This is because we haven't landed a patch to fix #4025. Also, the trace function will produce traces for each of the roots.

Result

Example output:

$ echo "WUT" >> BUILD.tools 
$ ./pants --enable-v2-engine list //:
Exception caught: (<class 'pants.build_graph.address_lookup_error.AddressLookupError'>)
  File "/.../pants/src/python/pants/bin/pants_exe.py", line 50, in <module>
    main()
  File "/.../pants/src/python/pants/bin/pants_exe.py", line 44, in main
    PantsRunner(exiter).run()
  File "/.../pants/src/python/pants/bin/pants_runner.py", line 57, in run
    options_bootstrapper=options_bootstrapper)
  File "/.../pants/src/python/pants/bin/pants_runner.py", line 46, in _run
    return LocalPantsRunner(exiter, args, env, options_bootstrapper=options_bootstrapper).run()
  File "/.../pants/src/python/pants/bin/local_pants_runner.py", line 37, in run
    self._run()
  File "/.../pants/src/python/pants/bin/local_pants_runner.py", line 77, in _run
    self._exiter).setup()
  File "/.../pants/src/python/pants/bin/goal_runner.py", line 184, in setup
    goals, context = self._setup_context()
  File "/.../pants/src/python/pants/bin/goal_runner.py", line 157, in _setup_context
    self._daemon_graph_helper
  File "/.../pants/src/python/pants/bin/goal_runner.py", line 101, in _init_graph
    graph, address_mapper = graph_helper.create_build_graph(target_roots, self._root_dir)
  File "/.../pants/src/python/pants/bin/engine_initializer.py", line 88, in create_build_graph
    for _ in graph.inject_specs_closure(target_roots.as_specs()):
  File "/.../pants/src/python/pants/engine/legacy/graph.py", line 205, in inject_specs_closure
    for address in self._inject(specs):
  File "/.../pants/src/python/pants/engine/legacy/graph.py", line 225, in _inject
    self._index(target_entries)
  File "/.../pants/src/python/pants/engine/legacy/graph.py", line 83, in _index
    'Build graph construction failed for {}:\n{}'.format(node, trace))

Exception message: Build graph construction failed for (SiblingAddresses(directory=u''), Select(Collection.of(HydratedTarget))):
Computing =Collection.of(HydratedTarget) for SiblingAddresses(directory=u'')
  Computing =Collection.of(HydratedTarget) for SiblingAddresses(directory=u'')
    Computing =HydratedTarget for SiblingAddresses(directory=u'')
      Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
        Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
          Computing =AddressFamily for SiblingAddresses(directory=u'')
            Computing =AddressFamily for Dir(path=u'')
              Computing =AddressFamily for Dir(path=u'')
                Throw("EntryId(17) failed!")

Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
  Computing =Collection.of(Address) for SiblingAddresses(directory=u'')
    Computing =AddressFamily for SiblingAddresses(directory=u'')
      Computing =AddressFamily for Dir(path=u'')
        Computing =AddressFamily for Dir(path=u'')
          Throw("EntryId(17) failed!")

Testing

Local manual testing with ./pants list and a corrupted BUILD file. Running tests that had been skipped due to this feature being missing. CI away on #4076

@baroquebobcat
Trace for the rust engine
f52476b 
Unfortunately, this still has some proof of concept bits in it. But, it does provide traces.

My caveat is that the traces do not include node information or the message from the failure. But you do see the subjects and the overall structure.

Additionally they print to stdout.
@baroquebobcat
move to temp file based flow
4c10cb9
@baroquebobcat
do something more like the original with cycles
e64bf52
@baroquebobcat
print unique traces for each root
d9241ca
@baroquebobcat
append to the files, since we're doing multiple traces, don't want to…
7e4b2ed 
… truncate

Also, add 4025 to skips where relevant
@baroquebobcat
add 4025 to another test skip note
0386a17
@baroquebobcat
further cleanup
fbdc295
@baroquebobcat
Merge branch 'master' into nhoward/trace_for_engine
44b0014 
Had a couple isort errors in ...tasks2. Fixed those.
@baroquebobcat
Followup for Stu's comments
e8ed3af
@baroquebobcat
bump native version
d9da4fb
@baroquebobcat
missed one last trace usage
073d984
@baroquebobcat baroquebobcat changed the title Trace for the rust engine [engine] Initial Trace implementation for rust engine Nov 19, 2016
@baroquebobcat
Copy link
Contributor Author

This patch was initially reviewed on https://rbcommons.com/s/twitter/r/4392/
@stuhood and @kwlzn gave ship its on it there. I pinged @benjyw, @JieGhost as reviewers as well.

@benjyw
Copy link
Contributor

benjyw commented Nov 19, 2016

I've been skipping over these because I know ~nothing about Rust.

On Fri, Nov 18, 2016 at 6:19 PM, Nick Howard [email protected]
wrote:

This patch was initially reviewed on https://rbcommons.com/s/
twitter/r/4392/
@stuhood https://github.com/stuhood and @kwlzn
https://github.com/kwlzn gave ship its on it there. I pinged @benjyw
https://github.com/benjyw, @JieGhost https://github.com/JieGhost as
reviewers as well.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4076 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAfS_Ir3xg1EIaiVl8zu74bPSO3mXWQMks5q_lyngaJpZM4K2pZC
.

stuhood
stuhood approved these changes Nov 19, 2016
View reviewed changes
src/rust/engine/src/scheduler.rs
@@ -45,6 +45,16 @@ impl Scheduler {
self.graph.visualize(&self.roots, path, &self.tasks.externs)
}

pub fn trace(&self, path: &Path) -> io::Result<()> {
for root in &self.roots {
let result = self.graph.trace(&root, path, &self.tasks.externs);
Copy link
Member

Choose a reason for hiding this comment

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

It should be equivalent to wrap this line in try!()... equivalent the new ? operator: https://blog.rust-lang.org/2016/11/10/Rust-1.13.html

@stuhood stuhood merged commit 37122df into pantsbuild:master Nov 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuhood stuhood stuhood approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Transfer Throw/Noop values across the native boundary
3 participants
@baroquebobcat @benjyw @stuhood