Skip to content

Update all dependencies (including nom 7.1.0) and update code base#27

Open
Titaniumtown wants to merge 10 commits intorekka:masterfrom
Titaniumtown:master
Open

Update all dependencies (including nom 7.1.0) and update code base#27
Titaniumtown wants to merge 10 commits intorekka:masterfrom
Titaniumtown:master

Conversation

@Titaniumtown
Copy link
Copy Markdown

@Titaniumtown Titaniumtown commented Feb 16, 2022

(Includes an updated version of #22 and #26)
This PR updates all dependencies (including nom, using updated fixes from #22) and fixes depreciation warnings (from #26). I hope this PR can serve as a meta PR of sorts as it seems development has ceased on this project.

@rekka
Copy link
Copy Markdown
Owner

rekka commented Feb 21, 2022

Thank you for the pull request. I currently am a bit short on time so it might take me a week or two to review this.

The development ceased since the crate does what I need it to do and I haven't had any issues using it in my project. I also forgot how to publish updates to crates.io in the meantime. :)

I have a few questions about this pull request:

  • Is there a reason why the update to nom 7 is necessary? Nom 1 seems to work fine.

  • Do the deprecation warnings show up when this crate is used as a dependency?

  • Why is it useful to specify the precise current version of serde? Isn't serde = { version = "1", optional = true } fine? Is there an anticipation that a minor version bump will break compatibility?

@Titaniumtown
Copy link
Copy Markdown
Author

Is there a reason why the update to nom 7 is necessary? Nom 1 seems to work fine.

I mean, doesn't hurt not to update it.

Do the deprecation warnings show up when this crate is used as a dependency?

I don't think so? I can't remember.

Why is it useful to specify the precise current version of serde? Isn't serde = { version = "1", optional = true } fine? Is there an anticipation that a minor version bump will break compatibility?

I was not aware it works that way. I'll revert that change later.

@MiscStuff839
Copy link
Copy Markdown

MiscStuff839 commented Feb 23, 2022

This PR updates all dependencies (including nom, using updated fixes from #22) and fixes depreciation warnings (from #22).

you mean depracation warnings by #26

@MiscStuff839
Copy link
Copy Markdown

MiscStuff839 commented Feb 23, 2022

Do the deprecation warnings show up when this crate is used as a dependency?

No it only appears when compiling the source but mainly the operators and the try! macro were depracated for more information see #26 (I refer to it everywhere because it is my first contribution to open-source so just ignore and pls let me enjoy in my bubble for a bit. thank you)

@Titaniumtown
Copy link
Copy Markdown
Author

This PR updates all dependencies (including nom, using updated fixes from #22) and fixes depreciation warnings (from #22).

you mean depracation warnings by #26

Whoops! Typo.

@Titaniumtown
Copy link
Copy Markdown
Author

@rekka Anything blocking this PR? Just wondering.

@MiscStuff839
Copy link
Copy Markdown

MiscStuff839 commented Mar 7, 2022

There do not seem to be any merge conflicts but why is the CI not running?

@Titaniumtown
Copy link
Copy Markdown
Author

No clue.

Copy link
Copy Markdown

@MiscStuff839 MiscStuff839 left a comment

Choose a reason for hiding this comment

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

Is there a reason that the values should not be constants seeing as they are hardcoded anyway?

@Titaniumtown
Copy link
Copy Markdown
Author

Is there a reason that the values should not be constants seeing as they are hardcoded anyway?

Do elaborate on what values you're referring to.

@MiscStuff839
Copy link
Copy Markdown

@rekka any issue with the PR?

@MiscStuff839
Copy link
Copy Markdown

Is there a reason that the values should not be constants seeing as they are hardcoded anyway?

Do elaborate on what values you're referring to.

Never mind after testing I understood.

@emixa-d
Copy link
Copy Markdown

emixa-d commented Aug 4, 2022

I tried out these patches but I'm getting some test failures:

starting phase `check'

running 13 tests
test expr::tests::hash_context ... ok
test expr::tests::test_builtins ... ok
test expr::tests::test_eval ... ok
test extra_math::tests::test_factorial ... ok
test expr::tests::test_eval_func_ctx ... ok
test shunting_yard::tests::test_to_rpn ... ok
test expr::tests::test_bind ... ok
test tokenizer::tests::test_func ... ok
test tokenizer::tests::test_var ... ok
test tokenizer::tests::test_tokenize ... ok
test tokenizer::tests::it_works ... FAILED
test tokenizer::tests::test_lexpr ... FAILED
test tokenizer::tests::test_number ... FAILED

failures:

---- tokenizer::tests::it_works stdout ----
thread 'tokenizer::tests::it_works' panicked at 'assertion failed: `(left == right)`
  left: `Err(Error(("+1.34e2", OneOf)))`,
 right: `Ok(("", Number(134.0)))`', src/tokenizer.rs:357:9
stack backtrace:
   0:     0x7ffff7e3361c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h7767eb774a4d4f42
   1:     0x7ffff7ec4bfc - core::fmt::write::hf3fb1fb91b38bb59
   2:     0x7ffff7e01bd5 - <unknown>
   3:     0x7ffff7e151bb - <unknown>
   4:     0x7ffff7e14d61 - <unknown>
   5:     0x7ffff7e1598b - std::panicking::rust_panic_with_hook::h19ec2b337ecb8872
   6:     0x7ffff7e34450 - <unknown>
   7:     0x7ffff7e33744 - <unknown>
   8:     0x7ffff7e15402 - rust_begin_unwind
   9:     0x7ffff7df5681 - core::panicking::panic_fmt::h964c785332841a13
  10:     0x7ffff7ea87f8 - core::panicking::assert_failed_inner::h45014c2f24de47a0
  11:     0x55555555b20b - core::panicking::assert_failed::hc09dc98f5734ad42
                               at /tmp/guix-build-rust-1.57.0.drv-0/rustc-1.57.0-src/library/core/src/panicking.rs:138:5
  12:     0x55555556c890 - meval::tokenizer::tests::it_works::h257179fc5ac459eb
                               at /tmp/guix-build-rust-meval-0.2.0.drv-0/source/src/tokenizer.rs:357:9
  13:     0x7ffff7f58293 - <unknown>
  14:     0x7ffff7f5845b - <unknown>
  15:     0x7ffff7f6df75 - <unknown>
  16:     0x7ffff7f66d0f - <unknown>
  17:     0x7ffff7e250e3 - <unknown>
  18:     0x7ffff7a58d7e - <unknown>
  19:     0x7ffff7b66eff - clone
  20:                0x0 - <unknown>

---- tokenizer::tests::test_lexpr stdout ----
thread 'tokenizer::tests::test_lexpr' panicked at 'assertion failed: `(left == right)`
  left: `Err(Error(("a", OneOf)))`,
 right: `Err(Error(("a", Float)))`', src/tokenizer.rs:378:9
stack backtrace:
   0:     0x7ffff7e3361c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h7767eb774a4d4f42
   1:     0x7ffff7ec4bfc - core::fmt::write::hf3fb1fb91b38bb59
   2:     0x7ffff7e01bd5 - <unknown>
   3:     0x7ffff7e151bb - <unknown>
   4:     0x7ffff7e14d61 - <unknown>
   5:     0x7ffff7e1598b - std::panicking::rust_panic_with_hook::h19ec2b337ecb8872
   6:     0x7ffff7e34450 - <unknown>
   7:     0x7ffff7e33744 - <unknown>
   8:     0x7ffff7e15402 - rust_begin_unwind
   9:     0x7ffff7df5681 - core::panicking::panic_fmt::h964c785332841a13
  10:     0x7ffff7ea87f8 - core::panicking::assert_failed_inner::h45014c2f24de47a0
  11:     0x55555555b20b - core::panicking::assert_failed::hc09dc98f5734ad42
                               at /tmp/guix-build-rust-1.57.0.drv-0/rustc-1.57.0-src/library/core/src/panicking.rs:138:5
  12:     0x55555556d28d - meval::tokenizer::tests::test_lexpr::h37dbe191a8f6f566
                               at /tmp/guix-build-rust-meval-0.2.0.drv-0/source/src/tokenizer.rs:378:9
  13:     0x7ffff7f58293 - <unknown>
  14:     0x7ffff7f5845b - <unknown>
  15:     0x7ffff7f6df75 - <unknown>
  16:     0x7ffff7f66d0f - <unknown>
  17:     0x7ffff7e250e3 - <unknown>
  18:     0x7ffff7a58d7e - <unknown>
  19:     0x7ffff7b66eff - clone
  20:                0x0 - <unknown>

---- tokenizer::tests::test_number stdout ----
thread 'tokenizer::tests::test_number' panicked at 'assertion failed: `(left == right)`
  left: `Err(Failure(("1E", Float)))`,
 right: `Err(Error(("E", Eof)))`', src/tokenizer.rs:460:9
stack backtrace:
   0:     0x7ffff7e3361c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h7767eb774a4d4f42
   1:     0x7ffff7ec4bfc - core::fmt::write::hf3fb1fb91b38bb59
   2:     0x7ffff7e01bd5 - <unknown>
   3:     0x7ffff7e151bb - <unknown>
   4:     0x7ffff7e14d61 - <unknown>
   5:     0x7ffff7e1598b - std::panicking::rust_panic_with_hook::h19ec2b337ecb8872
   6:     0x7ffff7e34450 - <unknown>
   7:     0x7ffff7e33744 - <unknown>
   8:     0x7ffff7e15402 - rust_begin_unwind
   9:     0x7ffff7df5681 - core::panicking::panic_fmt::h964c785332841a13
  10:     0x7ffff7ea87f8 - core::panicking::assert_failed_inner::h45014c2f24de47a0
  11:     0x55555555b20b - core::panicking::assert_failed::hc09dc98f5734ad42
                               at /tmp/guix-build-rust-1.57.0.drv-0/rustc-1.57.0-src/library/core/src/panicking.rs:138:5
  12:     0x55555556f30a - meval::tokenizer::tests::test_number::h9f3aa1bd07760254
                               at /tmp/guix-build-rust-meval-0.2.0.drv-0/source/src/tokenizer.rs:460:9
  13:     0x7ffff7f58293 - <unknown>
  14:     0x7ffff7f5845b - <unknown>
  15:     0x7ffff7f6df75 - <unknown>
  16:     0x7ffff7f66d0f - <unknown>
  17:     0x7ffff7e250e3 - <unknown>
  18:     0x7ffff7a58d7e - <unknown>
  19:     0x7ffff7b66eff - clone
  20:                0x0 - <unknown>


failures:
    tokenizer::tests::it_works
    tokenizer::tests::test_lexpr
    tokenizer::tests::test_number

test result: FAILED. 10 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

In case it matters, this is with rust 1.56, rust-serde-json 1.0.74, rust-serde-test 1.0.113, rust-fnv 1.0.6, rust-nom 7.1.1 and rust-serde 1.0.133.

@hydra
Copy link
Copy Markdown

hydra commented Jan 13, 2023

Any update on this? I'm trying to clear-up all the warnings in a project I'm working on and the 'meval' crate is a dependency of the 'textplots' crate, and thus I'm seeing a bunch of warnings relating to the very old nom 1.2.4 dependency so it would be excellent if this crate could be updated as per this PR.

@antimora
Copy link
Copy Markdown

@rekka Can you please merge this update and push a new release? We are getting a build warning telling that the future version of Rust will be rejected:

    Finished dev [unoptimized + debuginfo] target(s) in 0.30s
warning: the following packages contain code that will be rejected by a future version of Rust: nom v1.2.4

@Keavon
Copy link
Copy Markdown

Keavon commented Mar 27, 2024

@rekka I'd really appreciate if you could take a moment to merge this and deploy a new release. It causes warnings, and will eventually break entirely, because the old version of nom won't be valid Rust code in the future. An update would help us avoid this future problem and stop having to see the warning every time. Thank you!

@ognevny
Copy link
Copy Markdown

ognevny commented Jul 23, 2024

I'd really appreciate if you could take a moment to merge this and deploy a new release. It causes warnings, and will eventually break entirely, because the old version of nom won't be valid Rust code in the future. An update would help us avoid this future problem and stop having to see the warning every time. Thank you!

for now you all can only add [patch.crates-io] in manifest with this fork. personally I don't believe that anything will be changed in future ;(

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 this pull request may close these issues.

8 participants