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

Dinamically find radare2 install dir on Windows #14323

Merged
merged 8 commits into from Jun 20, 2019

Conversation

GustavoLCR
Copy link
Contributor

#10613
better #14315

Fixes types and etc not being loaded if radare2 is spawned outside of install dir

@@ -1143,7 +1143,21 @@ R_API bool r_sys_tts(const char *txt, bool bg) {
R_API const char *r_sys_prefix(const char *pfx) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should probably return char * in the heap here. this static char *thing will fail if we use more than one rsysprefix in the same function or so

Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

maybe do this change in a separate PR. also i dontn like this separation of windows and cutter, i dont see why cutter windows build should be different than the standard windows build

@GustavoLCR
Copy link
Contributor Author

maybe do this change in a separate PR. also i dontn like this separation of windows and cutter, i dont see why cutter windows build should be different than the standard windows build

cutter doesn't ship a radare2.exe, so if someone installs just cutter this would fail.

Also I think there is one more thing wrong with this. I just noticed that my install dir has a different structure from the one in appveyor, The exes are inside a bin folder on my system. Is this because of the --shared flag?

@codecov-io
Copy link

codecov-io commented Jun 16, 2019

Codecov Report

Merging #14323 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14323      +/-   ##
==========================================
- Coverage   37.85%   37.84%   -0.01%     
==========================================
  Files         948      948              
  Lines      305785   305802      +17     
==========================================
- Hits       115741   115739       -2     
- Misses     190044   190063      +19
Impacted Files Coverage Δ
libr/core/cconfig.c 88.56% <ø> (ø) ⬆️
libr/util/sys.c 52.57% <100%> (+0.13%) ⬆️
libr/core/project.c 57.47% <0%> (-0.34%) ⬇️
libr/core/panels.c 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ede4525...ec79e5e. Read the comment docs.

libr/util/sys.c Outdated Show resolved Hide resolved
libr/util/sys.c Show resolved Hide resolved
@GustavoLCR GustavoLCR changed the title Dinamically find radare2 install dir on Windows [WIP] Dinamically find radare2 install dir on Windows Jun 16, 2019
@GustavoLCR
Copy link
Contributor Author

I will check the broken tests

@GustavoLCR GustavoLCR changed the title [WIP] Dinamically find radare2 install dir on Windows Dinamically find radare2 install dir on Windows Jun 16, 2019
@GustavoLCR
Copy link
Contributor Author

OK, It's working now, but Appveyor is broken on master.

Also because of 977e8ef, I think the build script for Cutter and the Windows installer will have to add \bin to the PATH as well like 1b52bbf

@GustavoLCR
Copy link
Contributor Author

so is this whahts breaking the build?

@radare no, it isn't even merged on master, and it is master that's broken. I have no idea why. I cannot reproduce

Copy link
Contributor

@xarkes xarkes left a comment

Choose a reason for hiding this comment

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

Lgtm

@radare radare merged commit ae00b18 into radareorg:master Jun 20, 2019
@GustavoLCR GustavoLCR deleted the win-prefix branch June 21, 2019 16:21
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.

None yet

4 participants