Page MenuHomePhabricator

varun (Varun Dhananjaya)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 30 2021, 11:14 AM (43 w, 1 d)

Recent Activity

Fri, Sep 23

varun requested review of D5225: [services] create ecr repository for identity service docker images.
Fri, Sep 23, 1:22 PM
varun accepted D5223: [flow] Add `(killall flow || pkill flow || true)` to `cleaninstall`.
Fri, Sep 23, 12:22 PM
varun closed D5209: [services][identity] update user's public keys in ddb during wallet login.
Fri, Sep 23, 11:22 AM
varun committed rCOMM7153b39ed524: [services][identity] update user's public keys in ddb during wallet login (authored by varun).
[services][identity] update user's public keys in ddb during wallet login
Fri, Sep 23, 11:22 AM
varun closed D5200: [services][identity] change DDB schema for public keys to map.
Fri, Sep 23, 11:22 AM
varun closed D5208: [services][identity] update user's public keys in ddb during PAKE login.
Fri, Sep 23, 11:22 AM
varun committed rCOMMaa4790881d3b: [services][identity] update user's public keys in ddb during PAKE login (authored by varun).
[services][identity] update user's public keys in ddb during PAKE login
Fri, Sep 23, 11:22 AM
varun committed rCOMMc827b0ea0fcc: [services][identity] change DDB schema for public keys to map (authored by varun).
[services][identity] change DDB schema for public keys to map
Fri, Sep 23, 11:22 AM
varun closed D5217: [services][identity] address some nits.
Fri, Sep 23, 11:22 AM
varun committed rCOMMd6285ed30cad: [services][identity] address some nits (authored by varun).
[services][identity] address some nits
Fri, Sep 23, 11:22 AM

Thu, Sep 22

varun added inline comments to D5208: [services][identity] update user's public keys in ddb during PAKE login.
Thu, Sep 22, 1:54 PM
varun added inline comments to D5200: [services][identity] change DDB schema for public keys to map.
Thu, Sep 22, 1:53 PM
varun requested review of D5217: [services][identity] address some nits.
Thu, Sep 22, 12:45 PM
varun updated the diff for D5209: [services][identity] update user's public keys in ddb during wallet login.

remove unnecessary closure

Thu, Sep 22, 12:38 PM
varun updated the diff for D5208: [services][identity] update user's public keys in ddb during PAKE login.

remove unnecessary closure

Thu, Sep 22, 12:38 PM
varun updated the diff for D5200: [services][identity] change DDB schema for public keys to map.

rebase

Thu, Sep 22, 12:38 PM
varun added inline comments to D5200: [services][identity] change DDB schema for public keys to map.
Thu, Sep 22, 12:30 PM
varun planned changes to D5200: [services][identity] change DDB schema for public keys to map.
Thu, Sep 22, 12:16 PM
varun accepted D5214: [CI] Bump GH Actions `ios_ci` job to `macos-12`.
Thu, Sep 22, 7:11 AM
varun requested review of D5209: [services][identity] update user's public keys in ddb during wallet login.
Thu, Sep 22, 12:01 AM

Wed, Sep 21

varun requested review of D5208: [services][identity] update user's public keys in ddb during PAKE login.
Wed, Sep 21, 11:53 PM
varun requested review of D5200: [services][identity] change DDB schema for public keys to map.
Wed, Sep 21, 11:05 PM
varun added 1 blocking reviewer(s) for D4948: [Nix] Add rabbitmq-up command: max.

i think max should be blocking here since he knows the most about rabbit mq

Wed, Sep 21, 10:38 PM
varun accepted D5205: [services] Identity - Adding the generated protobuf files.

ah thanks. do we actually use these files anywhere though?

Wed, Sep 21, 10:37 PM
varun accepted D5154: [Services] Cleanup commtest/build.rs.

accepting with one comment

Wed, Sep 21, 2:24 PM
varun accepted D5204: [CI] Add `--network-timeout 180000` to `yarn` commands.
Wed, Sep 21, 12:13 PM

Tue, Sep 20

varun requested review of D5200: [services][identity] change DDB schema for public keys to map.
Tue, Sep 20, 9:59 PM
varun accepted D5196: [Nix] Add backwards compatible stub for older nix versions.
Tue, Sep 20, 12:10 PM

Mon, Sep 19

varun accepted D5053: Define tokio runtime.
Mon, Sep 19, 11:05 AM
varun accepted D5167: [Nix] Update flake structure.
Mon, Sep 19, 11:04 AM

Fri, Sep 16

varun closed D5168: [native] more refactoring of the native rust library.
Fri, Sep 16, 5:23 PM
varun committed rCOMM3b94e3aedfd1: [native] more refactoring of the native rust library (authored by varun).
[native] more refactoring of the native rust library
Fri, Sep 16, 5:23 PM
varun closed D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
Fri, Sep 16, 5:13 PM
varun committed rCOMMfee76b144596: [native] restructure gRPC client cargo project as general purpose library crate… (authored by varun).
[native] restructure gRPC client cargo project as general purpose library crate…
Fri, Sep 16, 5:13 PM
varun removed reviewers for D5168: [native] more refactoring of the native rust library: jon, atul, tomek.
Fri, Sep 16, 4:45 PM
varun removed a reviewer for D5104: [native] restructure gRPC client cargo project as general purpose library crate for native: atul.
Fri, Sep 16, 4:44 PM
varun added inline comments to D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
Fri, Sep 16, 4:42 PM
varun added inline comments to D5168: [native] more refactoring of the native rust library.
Fri, Sep 16, 4:39 PM
varun added inline comments to D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
Fri, Sep 16, 3:58 PM
varun added inline comments to D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
Fri, Sep 16, 3:55 PM
varun added inline comments to D5168: [native] more refactoring of the native rust library.
Fri, Sep 16, 3:52 PM
varun updated subscribers of D5168: [native] more refactoring of the native rust library.
Fri, Sep 16, 3:34 PM
varun added a comment to D5135: [CI] Add identity pre-commit hook.

could also use --manifest-path to avoid cd-ing

Fri, Sep 16, 3:31 PM
varun accepted D5135: [CI] Add identity pre-commit hook.
Fri, Sep 16, 3:29 PM
varun added a comment to D5053: Define tokio runtime.

unwrap is fine, but as @marcin said this diff should just be discarded or used to modify the runtime here

Fri, Sep 16, 3:27 PM
varun updated the summary of D5168: [native] more refactoring of the native rust library.
Fri, Sep 16, 3:14 PM
varun requested review of D5168: [native] more refactoring of the native rust library.
Fri, Sep 16, 3:06 PM
varun closed D5130: [services][identity] add public key to pake credential request.
Fri, Sep 16, 3:00 PM
varun committed rCOMM64ee37fedfdf: [services][identity] add public key to pake credential request (authored by varun).
[services][identity] add public key to pake credential request
Fri, Sep 16, 3:00 PM
varun added a comment to D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
In D5104#151281, @jon wrote:

If the output files are known ahead of time, we should probably be using a "build rule" rather than a "build phase". The difference being phases are ran regardless (so this script gets ran on every build), where as "build rules" can have the "the output is newer than the input" early exit behavior.

Fri, Sep 16, 2:53 PM
varun added inline comments to D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
Fri, Sep 16, 1:56 PM
varun updated the diff for D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.

Some more restructuring and added another script for cleanup (this was originally in Xcode but was unreadable there).

Fri, Sep 16, 12:40 PM
varun accepted D5162: [web] Change See More... hover color to match chat threads hover color.

resolves https://linear.app/comm/issue/ENG-1462/consistent-hover-state-for-sidebar-list-in-web-inbox

Fri, Sep 16, 11:48 AM
varun added inline comments to D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
Fri, Sep 16, 11:09 AM
varun updated the diff for D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.

address feedback

Fri, Sep 16, 11:06 AM

Thu, Sep 15

varun accepted D5147: [GH Actions] Add `pull_request` trigger to Actions workflows.
Thu, Sep 15, 2:55 PM

Wed, Sep 14

varun requested review of D5104: [native] restructure gRPC client cargo project as general purpose library crate for native.
Wed, Sep 14, 4:50 PM
varun accepted D4997: [landing] SIWE component with RainbowKit that connects & signs a message.

thanks for the explanation!

Wed, Sep 14, 10:30 AM
varun closed D5084: [native] add CXX header files to Xcode project.
Wed, Sep 14, 10:28 AM
varun committed rCOMM3e252805c510: [native] add CXX header files to Xcode project (authored by varun).
[native] add CXX header files to Xcode project
Wed, Sep 14, 10:28 AM
varun closed D5133: [GH Actions] Include `brew install protobuf` step in iOS build workflow.
Wed, Sep 14, 10:27 AM
varun committed rCOMM797a4b17afff: [GH Actions] Include `brew install protobuf` step in iOS build workflow (authored by atul).
[GH Actions] Include `brew install protobuf` step in iOS build workflow
Wed, Sep 14, 10:27 AM
varun commandeered D5133: [GH Actions] Include `brew install protobuf` step in iOS build workflow.
Wed, Sep 14, 10:25 AM
varun accepted D5133: [GH Actions] Include `brew install protobuf` step in iOS build workflow.

yeah, homebrew will install a version that works for the rust library, but won't work for our C++ gRPC codegen. this is fine for now I think. the right solution long-term is to install protobuf following the dev env setup instructions

Wed, Sep 14, 9:22 AM
varun added a comment to D5130: [services][identity] add public key to pake credential request.

these changes have to be made together to avoid CI issues

Wed, Sep 14, 12:50 AM
varun requested review of D5130: [services][identity] add public key to pake credential request.
Wed, Sep 14, 12:43 AM

Tue, Sep 13

varun updated the diff for D5084: [native] add CXX header files to Xcode project.

undo marking files as generated

Tue, Sep 13, 11:34 PM
varun added a comment to D5084: [native] add CXX header files to Xcode project.
In D5084#149061, @jon wrote:

i think this makes sense as a separate change because i'll be moving a bunch of files and it'll clutter this diff. what do you think?

with the generated file collapse, it will just be the paths which change. unless your manually editing the files.

Either way, the other revision which moved out the protos has landed, so I think we should avoid creating another directory.

Tue, Sep 13, 11:33 PM
varun planned changes to D5084: [native] add CXX header files to Xcode project.
In D5084#149188, @tomek wrote:
In D5084#149035, @varun wrote:

hmmm now the project.pbxproj file is showing up as generated on phabricator (which it is, but we still want phabricator to show it). is it because the word @generated appears?

Yes, that's the reason. We should use comm/shared/scripts/mark-generated.sh script instead of doing it directly.

Tue, Sep 13, 11:22 PM
varun requested changes to D4997: [landing] SIWE component with RainbowKit that connects & signs a message.
Tue, Sep 13, 11:13 PM
varun accepted D5128: [Nix] Provide c-ares cmake config.
Tue, Sep 13, 11:04 PM
varun closed D4953: [native] add Rust gRPC client to XCode project.
Tue, Sep 13, 10:52 PM
varun committed rCOMMaaf7ffa5fafe: [native] add Rust gRPC client to XCode project (authored by varun).
[native] add Rust gRPC client to XCode project
Tue, Sep 13, 10:52 PM
varun closed D5075: [native] remove dependency on identity service package for gRPC client.
Tue, Sep 13, 10:52 PM
varun committed rCOMMa7259c860c29: [native] remove dependency on identity service package for gRPC client (authored by varun).
[native] remove dependency on identity service package for gRPC client
Tue, Sep 13, 10:52 PM
varun added inline comments to D4953: [native] add Rust gRPC client to XCode project.
Tue, Sep 13, 10:46 PM
varun added a comment to D4953: [native] add Rust gRPC client to XCode project.

follow ups for both marcin and tomek's comments tracked here ^

Tue, Sep 13, 10:43 PM

Mon, Sep 12

varun added inline comments to D5075: [native] remove dependency on identity service package for gRPC client.
Mon, Sep 12, 3:24 PM

Fri, Sep 9

varun added a comment to D4953: [native] add Rust gRPC client to XCode project.
In D4953#149055, @jon wrote:

We should probably use a build rule so that it doesn't need to blindly call this each time. But blindly calling it isn't too terrible as cargo builds should be relatively quick after initial build

Fri, Sep 9, 2:10 PM
varun requested review of D5084: [native] add CXX header files to Xcode project.
In D5084#149052, @jon wrote:
Fri, Sep 9, 2:08 PM
varun added a comment to D5084: [native] add CXX header files to Xcode project.
In D5084#148472, @jon wrote:

I would like to avoid doing grpc/grpc_client and instead just have it under grpc/, after https://phab.comm.dev/D5045, only the client will remain in the folder

Fri, Sep 9, 12:47 PM
varun added a comment to D5084: [native] add CXX header files to Xcode project.

hmmm now the project.pbxproj file is showing up as generated on phabricator (which it is, but we still want phabricator to show it). is it because the word @generated appears?

Fri, Sep 9, 12:45 PM
varun updated the diff for D5084: [native] add CXX header files to Xcode project.

mark header files as generated

Fri, Sep 9, 12:40 PM
varun added a comment to D5084: [native] add CXX header files to Xcode project.
In D5084#148511, @atul wrote:
Unstaged changes in working copy:
    native/cpp/CommonCpp/grpc/grpc_client/cxx.h
    native/cpp/CommonCpp/grpc/grpc_client/lib.rs.h

Let's add these to .gitignore


(Accepting, but should address @jon's feedback and get his acceptance before landing)

Fri, Sep 9, 11:57 AM
varun updated the diff for D4953: [native] add Rust gRPC client to XCode project.

add cleanup build phase to remove the generated files. now we don't need to git ignore them

Fri, Sep 9, 11:55 AM
varun added a comment to D4953: [native] add Rust gRPC client to XCode project.

I modified my local branch with with Rust and C++ code for blob native client to mirror the setup presented above. When I attempt building Build Rust Library script was producing the following errors upon cargo build command:
{F164592}
Those errors are generated when cargo tries to compile cxx crate (the one we use for C++ bindings generation). This crate contains some C++ files and it looks like the are being compiled as if they were C++98 standard.
I diagnosed that cxx crate depends on cc crate. According to docs (https://crates.io/crates/cc) its behaviour can be modified by environmental variables. So I was able to successfully compile and run (exchange data with blob service) the whole project when I modified Build Rust Library script by adding export CXXFLAGS="-std=c++14" before cargo build and adding unset CXXFLAGS after cargo lipo.
I run git grep CXXFLAGS to see whether we set this flag somewhere in CI or else to understand why I needed to introduce such modification locally but everything works fine for CI. All I found was this file: services/tunnelbroker/docker/install_cryptopp.sh:make CXXFLAGS="-DNDEBUG -g2 -O3 -std=c++11". Not 100% sure but probably it is not relevant. @varun do you have an idea why settings this flag was necessary for me to compile project?

EDIT: I found the solution. When I run clang --version I get that I am using apple clang 13.1.6 which according to this: https://stackoverflow.com/questions/71670730/default-c-standard-is-c98-in-apple-clang-13 uses c++98 as default standard if not specified. So I can make a conclusion that it is my local issue which however can be experienced by someone else as well, who is using the same compiler. @varun can we safely add those lines? My local script looks like this:

# #!/bin/bash
set -x
# The $PATH used by Xcode likely won't contain Cargo, fix that.
# In addition, the $PATH used by XCode has lots of Apple-specific
# developer tools that your Cargo isn't expecting to use, fix that.
# Note: This assumes a default `rustup` setup and default path.
build_path="$HOME/.cargo/bin:/usr/local/bin:/usr/bin:/bin"
# cd to Cargo project
cd ${SRCROOT}/../cpp/CommonCpp/grpc/blob_client/rust
# Add iOS targets for cross-compilation
env PATH="${build_path}" rustup target add aarch64-apple-ios
env PATH="${build_path}" rustup target add x86_64-apple-ios
# Install cargo lipo
env PATH="${build_path}" cargo install cargo-lipo
# Build cxx bridge. Set C++ standard for C++ rust dependencies
export CXXFLAGS="-std=c++14"
env PATH="${build_path}" cargo build
# Build universal static library (works on simulator and iOS)
env PATH="${build_path}" cargo lipo --release "${build_args[@]}"
# unset the flag
unset CXXFLAGS
# Copy the .cc file to the cargo project root to make it available
# to XCode
cp $(readlink target/cxxbridge/rust/src/lib.rs.cc) .. #Type a script or drag a script file from your workspace to insert its path.
Fri, Sep 9, 9:45 AM
varun added a comment to D4953: [native] add Rust gRPC client to XCode project.
In D4953#148650, @tomek wrote:

I'm a little confused about the plan here. This diff references some files in xcode project that don't exist in the repo. When these files should be generated? When we checkout the code, is it in the correct state - no compilation errors?

The fact that we reference files that aren't in the repo and are ignored by git is confusing.

Fri, Sep 9, 7:40 AM

Thu, Sep 8

varun requested review of D5084: [native] add CXX header files to Xcode project.
Thu, Sep 8, 9:57 AM
varun added inline comments to D5075: [native] remove dependency on identity service package for gRPC client.
Thu, Sep 8, 9:56 AM
varun added a comment to D5075: [native] remove dependency on identity service package for gRPC client.
In D5075#148029, @atul wrote:

wrt Test Plan I can figure out where to run cargo build, but it's helpful for this sort of thing to give reviewers something they can copy and paste. Something like:

cd native/cpp/CommonCpp/grpc/grpc_client && cargo clean && cargo build
Thu, Sep 8, 9:54 AM
varun requested review of D4953: [native] add Rust gRPC client to XCode project.
In D4953#148020, @atul wrote:

Looks like we need to set $PROTOC for this to build. Can we add that to the dev environment setup and then send a message in Comm Dev Team with a note? Can accept right after that. Just don't want to break people's iOS environment by landing this before that.

(We'll probably need to make a similar change on the GH Actions side?)

Thu, Sep 8, 8:24 AM

Wed, Sep 7

varun added a comment to D4953: [native] add Rust gRPC client to XCode project.

https://medium.com/swiftblade/what-the-hell-is-this-output-files-in-xcode-build-phases-bfbec6391184

Wed, Sep 7, 12:23 PM
varun added a comment to D4953: [native] add Rust gRPC client to XCode project.
In D4953#147995, @atul wrote:

(Also let's add native/cpp/CommonCpp/grpc/grpc_client/target/ to .gitignore)

edit: also native/cpp/CommonCpp/grpc/grpc_client/lib.rs.cc if we aren't going to check it into the repo

Wed, Sep 7, 12:23 PM
varun updated the diff for D4953: [native] add Rust gRPC client to XCode project.

apparently the solution to our problem is simply specifying the output file. i cleaned my build folder, deleted the .cc file, and rebuilt. it seems to have worked.

Wed, Sep 7, 12:22 PM

Tue, Sep 6

varun added a comment to D4953: [native] add Rust gRPC client to XCode project.

nice this is finally passing CI

Tue, Sep 6, 1:39 PM
varun updated the diff for D4953: [native] add Rust gRPC client to XCode project.

update .gitignore, change readlink command

Tue, Sep 6, 1:28 PM
varun abandoned D4954: [docs] add instructions for installing cargo lipo.
Tue, Sep 6, 11:23 AM
varun updated the diff for D4953: [native] add Rust gRPC client to XCode project.

add target dir to .gitignore, add cross-compilation targets with rustup, edit generated file location

Tue, Sep 6, 11:19 AM
varun requested review of D5075: [native] remove dependency on identity service package for gRPC client.
Tue, Sep 6, 9:31 AM
varun added a comment to D4953: [native] add Rust gRPC client to XCode project.
Tue, Sep 6, 8:04 AM