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

Fresh CLI build not working with demo examples #358

Closed
conormurray95 opened this issue Feb 7, 2023 · 9 comments · Fixed by #359
Closed

Fresh CLI build not working with demo examples #358

conormurray95 opened this issue Feb 7, 2023 · 9 comments · Fixed by #359

Comments

@conormurray95
Copy link

Summary
I pulled the latest piranha codebase and attempted to compile and run the CLI binary against the feature_flag_cleanup/java example and couldn't get it to work using the documented inputs.

Steps to reproduce

  1. Added a piranha_arguments.toml file in demo/feature_flag_cleanup/java/configurations that looks like this:
language = ["java"]
substitutions = [
    ["stale_flag_name", "SAMPLE_STALE_FLAG"],
    ["treated", "true"]
]
  1. I added a Dockerfile that had the required dependencies of python and rust in the base directory of the repo. If you have a working local setup steps 2-7 can likely be skipped but I'll leave them here for ease of reproduction:
FROM python:3.9.16

COPY . ./piranha

# Install curl
RUN \
    apt update && \
    apt upgrade -y && \
    apt install -y curl


# Install rust \
RUN \
    curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > install_rust.sh && \
    sh install_rust.sh -y && \
    . "$HOME/.cargo/env"
  1. Build dockerfile docker build -t piranha:latest -f ./Dockerfile .
  2. Run docker image docker run -d -t piranha:latest
  3. docker exec -it ${CONTAINER_ID} /bin/bash
  4. cd piranha/
  5. Build binary: cargo build --release
  6. Run binary: target/release/polyglot_piranha --path-to-codebase demo/feature_flag_cleanup/java/ --path-to-configurations demo/feature_flag_cleanup/java/configurations/

Error
RUST_BACKTRACE=1 target/release/polyglot_piranha --path-to-codebase demo/feature_flag_cleanup/java/ --path-to-configurations demo/feature_flag_cleanup/java/configurations/

thread 'main' panicked at 'Language not supported', src/models/language.rs:187:12
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:64:14
   2: <polyglot_piranha::models::language::PiranhaLanguage as core::convert::From<&str>>::from
   3: polyglot_piranha::models::piranha_arguments::PiranhaArguments::merge
   4: polyglot_piranha::main

Full backtrace:

thread 'main' panicked at 'Language not supported', src/models/language.rs:187:12
stack backtrace:
   0:     0x55bcfcfe96ea - std::backtrace_rs::backtrace::libunwind::trace::h34aec3ef6cd8ad7e
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x55bcfcfe96ea - std::backtrace_rs::backtrace::trace_unsynchronized::h8035d38698d0f7a8
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55bcfcfe96ea - std::sys_common::backtrace::_print_fmt::hff968f6695a1ba22
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:65:5
   3:     0x55bcfcfe96ea - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h7eea0efe77acf1ec
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x55bcfd00b34e - core::fmt::write::hc553f17407eb0b48
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/fmt/mod.rs:1208:17
   5:     0x55bcfcfe6d15 - std::io::Write::write_fmt::h62e5f01a089f48c0
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/io/mod.rs:1682:15
   6:     0x55bcfcfe94b5 - std::sys_common::backtrace::_print::h52d116aff3db4cd1
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:47:5
   7:     0x55bcfcfe94b5 - std::sys_common::backtrace::print::h9e7d2f98fb7af075
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:34:9
   8:     0x55bcfcfeac9f - std::panicking::default_hook::{{closure}}::hf74999dab2d0a95c
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:267:22
   9:     0x55bcfcfea9db - std::panicking::default_hook::hc11ca7d10c44c42f
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:286:9
  10:     0x55bcfcfeb3ac - std::panicking::rust_panic_with_hook::hdb4da1ae79c845a5
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:688:13
  11:     0x55bcfcfeb102 - std::panicking::begin_panic_handler::{{closure}}::h02b5b35b126d5cf2
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:577:13
  12:     0x55bcfcfe9b9c - std::sys_common::backtrace::__rust_end_short_backtrace::h6c6853376cf416d1
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:137:18
  13:     0x55bcfcfeae52 - rust_begin_unwind
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:575:5
  14:     0x55bcfcd565a3 - core::panicking::panic_fmt::hfd9e949092070b66
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:64:14
  15:     0x55bcfcdbcb43 - <polyglot_piranha::models::language::PiranhaLanguage as core::convert::From<&str>>::from::h03f134d74c229209
  16:     0x55bcfcdace39 - polyglot_piranha::models::piranha_arguments::PiranhaArguments::merge::ha5b08eda6176640d
  17:     0x55bcfcd5f802 - polyglot_piranha::main::h983103ae54b037c1
  18:     0x55bcfcd5baf3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h4b88e154119a015d
  19:     0x55bcfcd5bb09 - std::rt::lang_start::{{closure}}::h3ab5e1c724ae26f5
  20:     0x55bcfcfe256c - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h9ab31282e87f134a
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/ops/function.rs:606:13
  21:     0x55bcfcfe256c - std::panicking::try::do_call::h42ddf5b01d0b4bc7
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:483:40
  22:     0x55bcfcfe256c - std::panicking::try::hfb70320d7386c61a
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:447:19
  23:     0x55bcfcfe256c - std::panic::catch_unwind::h978c9edbad2bb4d4
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panic.rs:137:14
  24:     0x55bcfcfe256c - std::rt::lang_start_internal::{{closure}}::h04ede5bd2f26b553
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:148:48
  25:     0x55bcfcfe256c - std::panicking::try::do_call::ha6b9da35a0885c93
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:483:40
  26:     0x55bcfcfe256c - std::panicking::try::h3325520cab3a642e
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:447:19
  27:     0x55bcfcfe256c - std::panic::catch_unwind::h160beec6f047175b
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panic.rs:137:14
  28:     0x55bcfcfe256c - std::rt::lang_start_internal::h79190e3a877a769d
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:148:20
  29:     0x55bcfcd5fd95 - main
  30:     0x7fc47926bd0a - __libc_start_main
  31:     0x55bcfcd56bca - _start
  32:                0x0 - <unknown>

Other Info
This error is the same if I delete piranha_arguments.toml file so I'm unsure if it's actually reading it at all.
I can successfully build the python version of polyglot-piranha and run using the script demo/stale_feature_flag_cleanup_demos.py.

@ketkarameya
Copy link
Contributor

ketkarameya commented Feb 7, 2023

Ow! I think i know why . Can you update your piranha_arguments.toml to :

language = "java"
substitutions = [
    ["stale_flag_name", "SAMPLE_STALE_FLAG"],
    ["treated", "true"]
]

Actually, I changed the type of language from list to unary.
f720a41#diff-2175302417330a100b6daa4d5e976745140033d8594d9f5194aa6fc087f9c4d7L76

To my knowledge I have updated all the tests/demos . Maybe I have not updated the README (yea I have not ) Sorry about that.

@conormurray95
Copy link
Author

Hey @ketkarameya thanks for following up. That actually gave back the same error again even using the string input for the language instead.

I also added the treated_complement to try and mimic the python script example here

language = "java"
substitutions = [
    ["stale_flag_name", "SAMPLE_STALE_FLAG"],
    ["treated", "true"],
    ["treated_complement", "false"]
]

@conormurray95
Copy link
Author

conormurray95 commented Feb 7, 2023

I just tested it against the find_replace_custom_cleanup/python/ demo example here seeing as it has a checked in piranha_arguments.toml to try and rule out some user error with formatting etc but got the same error again.

RUST_BACKTRACE=1 target/release/polyglot_piranha --path-to-codebase demo/find_replace_custom_cleanup/python/ --path-to-configurations demo/find_replace_custom_cleanup/python/configurations/
thread 'main' panicked at 'Language not supported', src/models/language.rs:187:12
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:64:14
   2: <polyglot_piranha::models::language::PiranhaLanguage as core::convert::From<&str>>::from
   3: polyglot_piranha::models::piranha_arguments::PiranhaArguments::merge
   4: polyglot_piranha::main

Seeing as this is the same error I get if no piranha_arguments file exists I wonder if it's either not being read at all or if the values are being replaced with empty defaults somehow in this PiranhaArguments::merge function.

Happy to test anything out if it helps.

@ketkarameya
Copy link
Contributor

Yea. There is a problem with PiranhaArguments. After the overhaul I have not accounted for the command line usecase throroughly. I ll get to it soon.
Thinking about it, we don't have tests for the command-line interface either. WE only have tests using the python/rust interface :|

@ketkarameya
Copy link
Contributor

ketkarameya commented Feb 8, 2023

@conormurray95 can you try this branch - fix-cli-bug

#359

@ketkarameya
Copy link
Contributor

The command has changed . You don't need to pass a piranha_arguments.toml

pass the language as -l java and substitutions as -s Foo=foo -s Bar=bar
Look here https://github.com/uber/piranha/pull/359/files#diff-bb03a17031af89adc46c263d1ab12a1341a5b3e671f2a2e3792a58be63134914R42 and the updated READEME in the PR

@conormurray95
Copy link
Author

Hey @ketkarameya that branch looks good for my CLI use case here. Tested by building with the Dockerfile ^ and running the java demo with this input:
target/release/polyglot_piranha --path-to-codebase demo/feature_flag_cleanup/java/ --path-to-configurations demo/feature_flag_cleanup/java/configurations/ -l java -s stale_flag_name=SAMPLE_STALE_FLAG -s treated=true -s treated_complement=false
and got the expected results.

Thanks for the quick updates!

@ketkarameya ketkarameya linked a pull request Feb 9, 2023 that will close this issue
@ketkarameya
Copy link
Contributor

@conormurray95 I am curious what is your specific use-case where you prefer using the cli over python/rust API ?

@conormurray95
Copy link
Author

@ketkarameya our team want to use the tool to cleanup flags but aren't rust/python developers so a cli tool seemed the more natural choice to get started, especially seeing as all of our other tooling e.g. gofmt, golint, goimports etc are all cli tools too. It was also to avoid any worries about installing dependencies and needing specific python versions to run it that might conflict with peoples existing setups.

In terms of how I've used it so far I packaged the binary up inside a minimal Dockerfile that we can run in CI or locally without installing any deps using a make command.

For ease of use for other developers it would be more convenient if the binary was available via something like homebrew that could be installed with one command and used via the cli. The reason I have it as a dockerfile right now is so our make flag-cleanup command can be run locally by any dev without any setup because it:

  • Pulls the docker image with Piranha tool installed
  • Mounts our codebase as a volume inside the image
  • Runs the piranha binary against the codebase using whatever flag they input

but really that's just a workaround so I can make it a one click install experience for others on our team.

All that being said I'm fairly new to the tool so if theres a better way to go about this I'd be all ears.

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 a pull request may close this issue.

2 participants