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

feat: add release build compilation #146

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

owittek
Copy link
Contributor

@owittek owittek commented Jul 17, 2023

Hey!

Personally I like to compile my nightly builds as a release for performance improvements. For that I have slightly refactored the code to check for this config option and to compile neovim nightly instead of downloading the pre-built binary in case this setting is enabled. It should also work for specific hash installations.

Currently Make has issues with those changes and returns the following:

/Library/Developer/CommandLineTools/usr/bin/make -C build install
[  0%] Built target update_version_stamp
[  4%] Built target nlua0
[ 91%] Built target nvim
[ 91%] Built target nvim_runtime_deps
[100%] Built target translations
[100%] Built target runtime
Install the project...
-- Install configuration: "Release"
Jul 16 22:54:15.055 ERROR Error: No such file or directory (os error 2)

I'll be investigating the cause later today in case this addition is even desired.

EDIT: My current suspicions are:

  • Line 274 as it's not cloning with --branch nightly (--branch actually matches tags too)
  • Cache issues (see bottom of this section)

@MordechaiHadad
Copy link
Owner

Looks really nice! what if we add another config property for making self built version, either release or RelWithDebugInfo?

@MordechaiHadad
Copy link
Owner

@owittek secondly, the error u are getting isn't from make at all, this formatting is from tracing which means an issue with bob

@owittek
Copy link
Contributor Author

owittek commented Jul 17, 2023

Looks really nice! what if we add another config property for making self built version, either release or RelWithDebugInfo?

Sure, I can look into that if it doesn't already work the way I implemented it

@owittek
Copy link
Contributor Author

owittek commented Jul 17, 2023

@owittek secondly, the error u are getting isn't from make at all, this formatting is from tracing which means an issue with bob

Makes sense. When this step fails my bob installation is also completely broken. It throws an error on every command that "bob.json could not be found" until I uninstall the failed version.

@MordechaiHadad
Copy link
Owner

btw @owittek did you fix the issue you got or are you still working on it?

@owittek
Copy link
Contributor Author

owittek commented Aug 5, 2023

btw @owittek did you fix the issue you got or are you still working on it?

Hey @MordechaiHadad I didn't get to work on this topic, I'll have to debug it when I find the time but feel free to try your luck in the mean time

@MordechaiHadad
Copy link
Owner

Yeah no problem, I am on a vacation anyway

@owittek
Copy link
Contributor Author

owittek commented Aug 13, 2023

I managed to debug the error. For some reason this line panics as the file creation fails. Do you have any idea why that could be the case with my changes? This error also happens with a normal nightly installation.

EDIT: My first guess was that it's a relative path issue so I installed my fork with cargo install --git https://github.com/owittek/bob.git but it still happens so I'm out of ideas without going through the entire source code.

@MordechaiHadad
Copy link
Owner

It seems like this damned file still gives issues, ill try to fix this tomorrow and then u could pull the changes

@owittek
Copy link
Contributor Author

owittek commented Aug 14, 2023

whoops, upon rebasing I inproperly resolved the merge conflict, will fix later..

@MordechaiHadad
Copy link
Owner

lol no problem

@owittek
Copy link
Contributor Author

owittek commented Aug 14, 2023

have you figured out the file issue in the mean-time? I tried debugging it but for some reason even the normal installation in my debugging container failed with the same issue. Is the bob.json just not being created properly?

@MordechaiHadad
Copy link
Owner

@owittek I havent checked it yesterday kinda got busy with another project, ill try today no promises tho

@owittek
Copy link
Contributor Author

owittek commented Aug 15, 2023

All good, I have just fixed my merge mistake so the code is actually usable now

@MordechaiHadad
Copy link
Owner

I pushed a new branch check if this fixes the issue.

@owittek owittek marked this pull request as ready for review August 16, 2023 19:35
@owittek
Copy link
Contributor Author

owittek commented Aug 16, 2023

I tried it and it works! Feel free to merge this PR after rebasing your fix to main (as your branch has diverged)

@owittek
Copy link
Contributor Author

owittek commented Aug 16, 2023

=)

image

@MordechaiHadad MordechaiHadad merged commit 771cce1 into MordechaiHadad:master Aug 17, 2023
8 of 9 checks passed
@MordechaiHadad
Copy link
Owner

ill release this tomorrow, thanks for the PR Oli!

@owittek
Copy link
Contributor Author

owittek commented Aug 17, 2023

I thank you for your cooperation 🫡

@owittek
Copy link
Contributor Author

owittek commented Aug 19, 2023

Hey @MordechaiHadad have you perhaps forgotten to release? 😁

@MordechaiHadad
Copy link
Owner

Nah I am just procrastinating, will release it tomorrow hopefully

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

2 participants