-
-
Notifications
You must be signed in to change notification settings - Fork 78
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 for attempting to remove a lib when dev is set #644
base: development
Are you sure you want to change the base?
fix for attempting to remove a lib when dev is set #644
Conversation
one thing this doesn't accomodate for yet, it should change the |
This PR doesn't seem to do what the description says 🤔 The codepath leading to the error you mentioned has not changed. Changing the behavior when removing the version used as "current" might not be what we want either, and implementation seems wrong anyway. |
hrmm I suppose what I'm trying to do is: when using a
So
|
src/haxelib/api/Repository.hx
Outdated
@@ -180,6 +181,9 @@ class Repository { | |||
} | |||
|
|||
FsUtils.deleteRec(versionPath); | |||
|
|||
var versions = getProjectInstallationInfo(name).versions.pop(); | |||
setCurrentVersion(name, versions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really don't want that. Especially since you're also doing it when removing a version that is not the current version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah I was incorrect with this one yes.
The intention was for the case of using a dev version and if you were to remove the current version, it doesn't change anything in the .current
file.
If you're on say flixel 5.0.0
(cat flixel/.current
-> 5.0.0
), begin using a dev directory for flixel, do something like haxelib remove flixel 5.0.0
, we want to properly update .current
so if you disable the dev, it leads to whatever the most recent directory is versions.pop()
.
However you're right, my implementation is incorrect since we don't want to remove / set the current version unconditionally!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does something like
FsUtils.deleteRec(versionPath);
if (current == version)
{
var versions = getProjectInstallationInfo(name).versions.pop();
setCurrentVersion(name, versions);
}
shake out? Unless I'm still misunderstanding 🙇♂️
In this snippet, the code will run if .dev
DOES exist (otherwise we would have thrown an error earlier). So if we have a .dev
haxelib, and we just removed our .current
set haxelib, we update the .current
file to the most recent version we have installed. Again, only if we are:
- uninstalling our "current" version
- we are using a
haxelib dev
based haxelib - there are other versions installed
haxelib dev [lib] path
haxelib remove [lib] git
Error: Library haxelib version git is not installed
currently
Repository.removeProjectVersion()
doesn't check if we're using a dev path, it only checks the.current
file. So here we check if we have a.dev
file (usinggetDevPath()
which returns null if no.dev
file is found)This PR just adds a simple check calling
getDevPath
to see if we are using a dev path, in which case we should be able to remove thegit
version of a haxelib