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

fix: windows path format in log4rs files #5234

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

brianp
Copy link
Contributor

@brianp brianp commented Mar 9, 2023

Description

This commit does a naive character replacement on the string path just before we do the path injection.

Motivation and Context

log4rs expects the paths to be in a unix format regardless of the system it's running on. When we started doing the dynamic injecting of the base path windows systems injected using a windows format, which mixed with the unix format. Resulting in strings that looked liked this: C:\\brianp\.tari\config/log/base-node/network.log

when log4rs wants the final format to be like this: C://brianp/.tari/config/log/base-node/network.log

How Has This Been Tested?

Manually on Hansie's windows machine

Co-authored-by: hansieodendaal 39146854+hansieodendaal@users.noreply.github.com

log4rs expects the paths to be in a unix format regardless of the system
it's running on. When we started doing the dynamic injecting of the base
path windows systems injected using a windows format, which mixed with
the unix format. Resulting in strings that looked liked this:
`C:\\brianp\.tari\config/log/base-node/network.log`

when it log4rs wants the final format to be like this:
`C://brianp/.tari/config/log/base-node/network.log`

This commit does a naive character replacement on the string path just
before we do the path injection.

Co-authored-by: hansieodendaal 39146854+hansieodendaal@users.noreply.github.com
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

hah weird - LGTM

@stringhandler stringhandler merged commit acfecfb into tari-project:development Mar 15, 2023
stringhandler pushed a commit that referenced this pull request Mar 15, 2023
### ⚠ BREAKING CHANGES

* **wallet:** ensure burn shared keys and hashes match dan layer (5245)
* add claim public key to OutputFeatures (5239)
* reset esmeralda (5247)

### Features

* add claim public key to OutputFeatures
([5239](#5239))
([3e7d82c](3e7d82c))
* reset esmeralda
([5247](#5247))
([aa2a3ad](aa2a3ad))


### Bug Fixes

* added transaction revalidation to the wallet startup sequence
[5227](#5227)
([5246](#5246))
([7b4e2d2](7b4e2d2))
* immediately fail to compile on 32-bit systems
([5237](#5237))
([76aeed7](76aeed7))
* **wallet:** correct change checks in transaction builder
([5235](#5235))
([768a0cf](768a0cf))
* **wallet:** ensure burn shared keys and hashes match dan layer
([5245](#5245))
([024ce64](024ce64))
* windows path format in log4rs files
([5234](#5234))
([acfecfb](acfecfb))
@brianp brianp deleted the fix-windows-log-path branch October 2, 2023 14:31
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.

3 participants