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

Introduce a logging system. #237

Merged
merged 2 commits into from
Jun 15, 2019
Merged

Introduce a logging system. #237

merged 2 commits into from
Jun 15, 2019

Conversation

achan1989
Copy link
Contributor

This pull request makes progress on #183.

I've replaced the existing print/eprint calls with the appropriate logging macros, and arbitrarily chose env_logger to handle the logging.

This should be functionally identical to the branch point, although the format of the logging output is different -- it's now of the form <SEVERITY> svd2rust: <message>.

@ryankurte
Copy link

bors try

bors bot added a commit that referenced this pull request Sep 28, 2018
@bors
Copy link
Contributor

bors bot commented Sep 28, 2018

try

Build failed

@achan1989
Copy link
Contributor Author

I think that might be an issue in the cortex-m-rt crate, introduced between versions 0.6.3 and 0.6.4? This older build managed to compile cortex-m-rt 0.6.3

@ryankurte
Copy link

Hmm, it does look like it is somewhere there, I'm unfortunately not knowledgeable enough about macros to fix it, but I think I've tracked down the problem. Also, thanks for opening the PR!

Travis Error when running TRAVIS_OS_NAME=linux TARGET=x86_64-unknown-linux-gnu VENDOR=Nordic ci/script.sh:

error[E0432]: unresolved import `macros::entry`
   --> /home/travis/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/cortex-m-rt-0.6.4/src/lib.rs:397:18
    |
397 | pub use macros::{entry, exception, pre_init};
    |                  ^^^^^ no `entry` in the root
error[E0432]: unresolved import `macros::exception`
   --> /home/travis/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/cortex-m-rt-0.6.4/src/lib.rs:397:25
    |
397 | pub use macros::{entry, exception, pre_init};
    |                         ^^^^^^^^^ no `exception` in the root
error[E0432]: unresolved import `macros::pre_init`
   --> /home/travis/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/cortex-m-rt-0.6.4/src/lib.rs:397:36
    |
397 | pub use macros::{entry, exception, pre_init};
    |                                    ^^^^^^^^ no `pre_init` in the root
error: aborting due to 3 previous errors

cortex-m-rt#L391:

extern crate cortex_m_rt_macros as macros;
...
pub use macros::{entry, exception, pre_init};

I have reproduced the issue on my machine using TARGET=x86_64-apple-darwin VENDOR=Nordic TRAVIS_OS_NAME=osx bash ci/script.sh building with cargo 1.29.0 (524a578d7 2018-08-05) and confirmed to be using the packages cortex-m-rt v0.6.4 and cortex-m-rt-macros v0.1.2 as in the failed test.

The issue does not seem occur with cargo 1.29.0-nightly (6a7672ef5 2018-08-14) or newer, it seems to me that the changes to cortex-m v0.6.4 requires newer language features.

@japaric does this seem like a reasonable conclusion / do you have any suggestions on how to fix it?

@mathk mathk requested review from japaric and ryankurte February 22, 2019 09:23
@mathk mathk added the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. label Feb 22, 2019
@japaric
Copy link
Member

japaric commented Feb 24, 2019

TRAVIS_OS_NAME=linux TARGET=x86_64-unknown-linux-gnu VENDOR=Nordic sh ci/script.sh works fine for me on this PR after rebasing, both on stable and nightly.

@mathk
Copy link

mathk commented Feb 25, 2019

bors try

@bors
Copy link
Contributor

bors bot commented Feb 25, 2019

try

Merge conflict

@mathk mathk added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Feb 25, 2019
@mathk
Copy link

mathk commented Feb 25, 2019

@achan1989 can you do a rebase/merge? Thanks

@therealprof
Copy link
Contributor

@achan1989 Sorry for the long delay. Are you still interested in this? If so would you please update the PR?

@achan1989
Copy link
Contributor Author

Oops. It's been a while, but I'll try to update.

svd2rust now uses env_logger instead of printing directly to stderr.
env_logger is currently configured to log messages >= info to stderr.
@achan1989 achan1989 requested a review from a team as a code owner June 1, 2019 10:28
@achan1989
Copy link
Contributor Author

bors try

@bors
Copy link
Contributor

bors bot commented Jun 1, 2019

🔒 Permission denied

Existing reviewers: click here to make achan1989 a reviewer

@ryankurte
Copy link

bors try

bors bot added a commit that referenced this pull request Jun 15, 2019
@bors
Copy link
Contributor

bors bot commented Jun 15, 2019

try

Build succeeded

@ryankurte
Copy link

bors r+

bors bot added a commit that referenced this pull request Jun 15, 2019
237: Introduce a logging system. r=ryankurte a=achan1989

This pull request makes progress on #183.

I've replaced the existing print/eprint calls with the appropriate logging macros, and arbitrarily chose env_logger to handle the logging.

This should be functionally identical to the branch point, although the format of the logging output is different -- it's now of the form `<SEVERITY> svd2rust: <message>`.

Co-authored-by: achan1989 <achan1989@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jun 15, 2019

Build succeeded

@bors bors bot merged commit 6b8f453 into rust-embedded:master Jun 15, 2019
@burrbull burrbull mentioned this pull request Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants