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

publish python commons eggs #3

Closed
jsirois opened this issue Jul 30, 2013 · 2 comments
Closed

publish python commons eggs #3

jsirois opened this issue Jul 30, 2013 · 2 comments
Assignees

Comments

@jsirois
Copy link
Contributor

jsirois commented Jul 30, 2013

This step will allow slicing the pants code out of twitter-commons into the pantsbuild repo. Changes to python commons will go through twitter-commons and be picked up with requirement upgrades.

@ghost ghost assigned jsirois Jul 30, 2013
@ericzundel
Copy link
Member

I think this has been done a long time ago. close?

@jsirois
Copy link
Contributor Author

jsirois commented Feb 24, 2015

Yes - long since done.

@jsirois jsirois closed this as completed Feb 24, 2015
kwlzn added a commit that referenced this issue Mar 31, 2017
…n test. (#4407)

### Problem

Currently, on Linux the first thin client call to the daemon can deadlock just after the pantsd->fork->pantsd-runner workflow. Connecting to the process with `gdb` reveals a deadlock in the following stack in the `post_fork` `drop` of the `CpuPool`:

```
#0  0x00007f63f04c31bd in __lll_lock_wait () from /lib64/libpthread.so.0
No symbol table info available.
#1  0x00007f63f04c0ded in pthread_cond_signal@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
No symbol table info available.
#2  0x00007f63d3cfa438 in notify_one ()
    at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys/unix/condvar.rs:52
No locals.
#3  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys_common/condvar.rs:39
No locals.
#4  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sync/condvar.rs:208
No locals.
#5  std::thread::{{impl}}::unpark () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/thread/mod.rs:633
No locals.
#6  0x00007f63d3c583d1 in crossbeam::sync::ms_queue::{{impl}}::push<futures_cpupool::Message> (self=<optimized out>, t=...)
    at /home/kwilson/.cache/pants/rust-toolchain/registry/src/github.com-1ecc6299db9ec823/crossbeam-0.2.10/src/sync/ms_queue.rs:178
        guard = <optimized out>
        self = <optimized out>
#7  0x00007f63d3c588ed in futures_cpupool::{{impl}}::drop (self=<optimized out>)
    at /home/kwilson/.cache/pants/rust-toolchain/git/checkouts/futures-rs-a4f11d094efefb0a/f7e6bc8/futures-cpupool/src/lib.rs:236
        self = 0x37547a0
#8  0x00007f63d3be871c in engine::fs::{{impl}}::post_fork (self=0x3754778) at /home/kwilson/dev/pants/src/rust/engine/src/fs.rs:355
        self = 0x3754778
#9  0x00007f63d3be10e4 in engine::context::{{impl}}::post_fork (self=0x37545b0)
    at /home/kwilson/dev/pants/src/rust/engine/src/context.rs:93
        self = 0x37545b0
#10 0x00007f63d3c0de5a in {{closure}} (scheduler=<optimized out>) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:275
        scheduler = 0x3740580
#11 with_scheduler<closure,()> (scheduler_ptr=<optimized out>, f=...) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:584
        scheduler = 0x3740580
        scheduler_ptr = 0x3740580
#12 engine::scheduler_post_fork (scheduler_ptr=0x3740580) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:274
        scheduler_ptr = 0x3740580
#13 0x00007f63d3c1be8c in _cffi_f_scheduler_post_fork (self=<optimized out>, arg0=0x35798f0) at src/cffi/native_engine.c:2234
        _save = 0x34a65a0
        x0 = 0x3740580
        datasize = <optimized out>
#14 0x00007f63f07b5a62 in PyEval_EvalFrameEx () from /lib64/libpython2.7.so.1.0
```

This presents as a hang in the thin client, because the pailgun socket is left open in the pantsd-runner.

### Solution

Add pre-fork hooks and tear down the `CpuPool` instances prior to forking and rebuilding them.

### Result

Can no longer reproduce the hang.
lenucksi pushed a commit to lenucksi/pants that referenced this issue Apr 25, 2017
…n test. (pantsbuild#4407)

### Problem

Currently, on Linux the first thin client call to the daemon can deadlock just after the pantsd->fork->pantsd-runner workflow. Connecting to the process with `gdb` reveals a deadlock in the following stack in the `post_fork` `drop` of the `CpuPool`:

```
#0  0x00007f63f04c31bd in __lll_lock_wait () from /lib64/libpthread.so.0
No symbol table info available.
pantsbuild#1  0x00007f63f04c0ded in pthread_cond_signal@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
No symbol table info available.
pantsbuild#2  0x00007f63d3cfa438 in notify_one ()
    at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys/unix/condvar.rs:52
No locals.
pantsbuild#3  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys_common/condvar.rs:39
No locals.
pantsbuild#4  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sync/condvar.rs:208
No locals.
pantsbuild#5  std::thread::{{impl}}::unpark () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/thread/mod.rs:633
No locals.
pantsbuild#6  0x00007f63d3c583d1 in crossbeam::sync::ms_queue::{{impl}}::push<futures_cpupool::Message> (self=<optimized out>, t=...)
    at /home/kwilson/.cache/pants/rust-toolchain/registry/src/github.com-1ecc6299db9ec823/crossbeam-0.2.10/src/sync/ms_queue.rs:178
        guard = <optimized out>
        self = <optimized out>
pantsbuild#7  0x00007f63d3c588ed in futures_cpupool::{{impl}}::drop (self=<optimized out>)
    at /home/kwilson/.cache/pants/rust-toolchain/git/checkouts/futures-rs-a4f11d094efefb0a/f7e6bc8/futures-cpupool/src/lib.rs:236
        self = 0x37547a0
pantsbuild#8  0x00007f63d3be871c in engine::fs::{{impl}}::post_fork (self=0x3754778) at /home/kwilson/dev/pants/src/rust/engine/src/fs.rs:355
        self = 0x3754778
pantsbuild#9  0x00007f63d3be10e4 in engine::context::{{impl}}::post_fork (self=0x37545b0)
    at /home/kwilson/dev/pants/src/rust/engine/src/context.rs:93
        self = 0x37545b0
pantsbuild#10 0x00007f63d3c0de5a in {{closure}} (scheduler=<optimized out>) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:275
        scheduler = 0x3740580
pantsbuild#11 with_scheduler<closure,()> (scheduler_ptr=<optimized out>, f=...) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:584
        scheduler = 0x3740580
        scheduler_ptr = 0x3740580
pantsbuild#12 engine::scheduler_post_fork (scheduler_ptr=0x3740580) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:274
        scheduler_ptr = 0x3740580
pantsbuild#13 0x00007f63d3c1be8c in _cffi_f_scheduler_post_fork (self=<optimized out>, arg0=0x35798f0) at src/cffi/native_engine.c:2234
        _save = 0x34a65a0
        x0 = 0x3740580
        datasize = <optimized out>
pantsbuild#14 0x00007f63f07b5a62 in PyEval_EvalFrameEx () from /lib64/libpython2.7.so.1.0
```

This presents as a hang in the thin client, because the pailgun socket is left open in the pantsd-runner.

### Solution

Add pre-fork hooks and tear down the `CpuPool` instances prior to forking and rebuilding them.

### Result

Can no longer reproduce the hang.
Eric-Arellano referenced this issue in Eric-Arellano/pants Jun 29, 2019
Eric-Arellano referenced this issue in Eric-Arellano/pants Jun 29, 2019
Eric-Arellano referenced this issue in Eric-Arellano/pants Jul 20, 2019
Eric-Arellano added a commit that referenced this issue Jan 10, 2020
Any time that you add a new top-level goal in V2—that is a "verb" for Pants like `test`, `fmt`, `setup-py`, and `dependencies`—you must define the same three basic things:

```
Goal, GoalSubsystem, @console_rule
```

One of those does not fit with the other two. In contrast, these three all belong:

```
Goal, GoalSubsystem, @goal_rule
```

### What is an `@console_rule`?

There are (at least) 3 ways of conceptualizing what an `@console_rule` means:

1. A rule that has exclusive access to the `Console`.
    * Only `@console_rules` can write to stdout and stderr.
    * This property is represented in the name `@console_rule`.
2. A rule that is not cached.
    * This is an important property of `@console_rules` and contrasts with normal `@rules`.
    * Per the conversation in #8931 (comment), this idea on uncachability is the main mental model for some.
3. A rule that maps 1-1 with a top-level goal / returns an `engine.goal.Goal`.

All three of these properties are true and important components of an `@console_rule`. However, I argue that #3 is the most important: _fundamentally, an `@console_rule` is a special rule that hooks up to a Pants goal_.

Right now, the name `@console_rule` optimizes for property #1. In contrast, `@goal_rule` focuses on property #3. 

(NB: all three of these properties will continue applying regardless of the name)

### Leveraging prior Pants knowledge

Most Pants users are familiar with the idea of a goal. It's the [very first concept introduced in our docs](https://www.pantsbuild.org/first_concepts.html) and every single Pants run involves invoking goals, regardless of V1 vs. V2.

In contrast, fewer users are familiar with the `Console` abstraction, which is never mentioned in our docs and is more of an implementation detail for how V2 Pants works.

We do expect users to soon write their own rules in plugins. The name `@goal_rule` will allow them to leverage their prior knowledge, just as `subsystem_rule` leverages prior knowledge better than `optionable_rule` did.
Eric-Arellano added a commit that referenced this issue Apr 5, 2020
Twice now, we've had `Sources` fields that require a specific number of files. So, this PR formalizes a mechanism to do this.

Error message #1:

> ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 3 files, but it had 1 file.

Error message #2: 

> ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 2 or 3 files, but it had 1 file.

Error message #3: 

> ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have a number of files in the range `range(20, 20000, 10)`, but it had 1 file.


[ci skip-rust-tests]  # No Rust changes made.
[ci skip-jvm-tests]  # No JVM changes made.
Eric-Arellano added a commit that referenced this issue Apr 28, 2020
## Goals of design

See https://docs.google.com/document/d/1tJ1SL3URSXUWlrN-GJ1fA1M4jm8zqcaodBWghBWrRWM/edit?ts=5ea310fd for more info. 

tl;dr:

1) Protocols now only have one generic target, like `avro_library`. This means that call sites must declare which language should be generated from that protocol.
    * Must be declarative.
2) You can still get the original protocol sources, e.g. for `./pants filedeps`.
3) Must work with subclassing of fields.
4) Must be extensible.
     * Example: Pants only implements Thrift -> Python. A plugin author should be able to add Thrift -> Java.

## Implementation

Normally, to hydrate sources, we call `await Get[HydratedSources](HydrateSourcesRequest(my_sources_field))`. We always use the exact same rule to do this because all `sources` fields are hydrated identically.

Here, each codegen rule is unique. So, we need to use unions. This means that we also need a uniform product for each codegen rule for the union to work properly. This leads to:

```python
await Get[GeneratedSources](GenerateSourcesRequest, GeneratePythonFromAvroRequest(..))
await Get[GeneratedSources](GenerateSourcesRequest, GenerateJavaFromThriftRequest(..))
```

Each `GenerateSourcesRequest` subclass gets registered as a union rule. This achieves goal #4 of extensibility.

--

To still work with subclassing of fields (goal #3), each `GenerateSourcesRequest` declares the input type and output type, which then allows us to use `isinstance()` to accommodate subclasses:

```python
class GenerateFortranFromAvroRequest(GenerateSourcesRequest):
    input = AvroSources
    output = FortranSources
```

--

To achieve goals #1 and #2 of allowing call sites to declaratively either get the original protocol sources or generated sources, we hook up codegen to the `hydrate_sources` rule and `HydrateSourcesRequest` type:

```python
protocol_sources = await Get[HydratedSources](HydrateSourcesRequest(avro_sources, for_sources_types=[FortranSources], codegen_enabled=True))
```

[ci skip-rust-tests]
[ci skip-jvm-tests]
olafurpg added a commit to olafurpg/pants that referenced this issue Sep 25, 2020
Bump rust-toolchain to version 1.46.0
jsirois added a commit to jsirois/pants that referenced this issue May 12, 2021
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
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

No branches or pull requests

2 participants