Skip to content

Add ldd --version check#10

Merged
GeckoEidechse merged 32 commits into
R2NorthstarTools:mainfrom
TH3-S4LM0N:main
Oct 18, 2022
Merged

Add ldd --version check#10
GeckoEidechse merged 32 commits into
R2NorthstarTools:mainfrom
TH3-S4LM0N:main

Conversation

@TH3-S4LM0N

Copy link
Copy Markdown
Contributor

hahhah finally the button works

Added a function in linux.rs to check ldd --version >= 2.33, but rn its inside of a general linux_checks function. Returns a bool so that anything about checking linux compatibility in the future can just be 1 button.

@GeckoEidechse GeckoEidechse self-assigned this Oct 6, 2022

@GeckoEidechse GeckoEidechse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR <3

Noticed a few things and annotated them accordingly. Need to test compile on Windows still but if it does (and the annotated things are resolved) should be good to merge ^^

I assume it worked on your machine?

Comment thread .gitignore Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-tauri/src/main.rs Outdated
Comment thread src-tauri/src/lib.rs Outdated
Comment thread src-tauri/src/platform_specific/linux.rs Outdated

@Alystrasz Alystrasz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lacks some comments and features some typos

Comment thread src-tauri/src/platform_specific/linux.rs Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-tauri/src/main.rs Outdated
Comment thread src-tauri/Cargo.toml Outdated
Comment thread src-tauri/tauri.conf.json
@GeckoEidechse GeckoEidechse mentioned this pull request Oct 7, 2022
100 tasks
Comment thread .github/workflows/push-test.yml Outdated
Comment thread .github/workflows/push-test.yml Outdated
@TH3-S4LM0N

Copy link
Copy Markdown
Contributor Author

Ok I think all review requests have been satisfied. It built and ran on both linux and windows on my machine, since it's still in the dev view it's not implemented. Next I'll prolly do nsproton and work this all into that.

@Alystrasz Alystrasz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Typos, typos everywhere

Comment thread src-tauri/src/lib.rs Outdated
Comment thread src-tauri/src/lib.rs Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread src-vue/src/views/DeveloperView.vue Outdated
Comment thread .gitignore Outdated
Comment thread src-tauri/tauri.conf.json Outdated
Comment thread .gitignore Outdated
Comment thread .gitignore Outdated

@GeckoEidechse GeckoEidechse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't break Windows and I assume Linux was tested, so fine to merge by me ^^

feat: Show own version number in settings view (#11)
Comment thread src-tauri/src/main.rs

@GeckoEidechse GeckoEidechse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So it works on Linux now without crash :D

But Windows fails to compile now atm ._.

Comment thread package-lock.json
Comment thread src-tauri/Cargo.toml Outdated
serde_json = "1.0"
serde = { version = "1.0", features = ["derive"] }
tauri = { version = "1.1.1", features = ["api-all", "updater"] }
tauri = { version = "1.1.1", features = ["api-all"] }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh keep forgetting this

Comment thread src-tauri/Cargo.lock
Comment on lines -1773 to -1778
[[package]]
name = "minisign-verify"
version = "0.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "933dca44d65cdd53b355d0b73d380a2ff5da71f87f036053188bf1eab6a19881"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

btw, why is Cargo.lock not in the gitignore?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It allows developers to have exact same versions of dependencies

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's the same as NPM and package-lock.json. Locks dependency to certain version for reproducible builds.

Comment thread src-tauri/src/main.rs Outdated
Comment thread src-tauri/src/main.rs
Comment on lines +123 to +129
fn linux_checks() -> bool {
if get_host_os() == "windows" {
false
} else {
linux_checks_librs()
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should probably be changed to returning a Result<T,E> type in the future but fine as is for now for me ^^

@GeckoEidechse

Copy link
Copy Markdown
Member

Tested working on Linux, will merge if CI passes.

@GeckoEidechse GeckoEidechse merged commit c4ce52b into R2NorthstarTools:main Oct 18, 2022
@GeckoEidechse GeckoEidechse removed their assignment Sep 24, 2024
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