Skip to content

use int compare when both version fields are ints #140

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

use int compare when both version fields are ints #140

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
87e86e9
use int compare when both version fields are ints
benprew May 10, 2025
80c5a2d
adhere to Debian version comparison guidelines
camilajenny Jun 20, 2025
2ed2b00
add the ValaUnit subproject for unit tests
camilajenny Jul 2, 2025
2ed21f3
move the compare_versions method to EddyUtils.vala
camilajenny Jul 2, 2025
d37f9ca
add unit tests for compare_versions
camilajenny Jul 2, 2025
f0f30de
add instructions on how to run unit tests to README.md
camilajenny Jul 2, 2025
c11716a
fix the upgrade / downgrade flag assignment depending on the version …
camilajenny Jul 2, 2025
ab4a8ef
add .idea to .gitignore
camilajenny Jul 2, 2025
7404aeb
Merge pull request #1 from camilajenny/master
benprew Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions src/Package.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,31 +32,31 @@ public class Eddy.Package : Object {
public signal void state_updated ();

public string filename { public get; construct; }
public string name {
public string name {
get {
return target.get_name ();
}
}

public string summary {
public string summary {
get {
return target.summary;
}
}

public string version {
public string version {
get {
return target.get_version ();
}
}

public uint64 installed_size {
public uint64 installed_size {
get {
return target.size;
}
}

public string homepage {
public string homepage {
owned get {
return target.url;
}
Expand All @@ -76,8 +76,8 @@ public class Eddy.Package : Object {
public bool has_task { get; set; default = false; }

public StateFlags state_flags { public get; private set; default = StateFlags.NOT_INSTALLED; }
public bool is_installed {

public bool is_installed {
get {
return StateFlags.INSTALLED in state_flags;
}
Expand Down Expand Up @@ -178,7 +178,7 @@ public class Eddy.Package : Object {
foreach (var package in packages) {
yield package.reset ();
package.exit_code = exit_code;
}
}

return result;
}
Expand Down Expand Up @@ -282,19 +282,19 @@ public class Eddy.Package : Object {

int length = int.max (aparts.length, bparts.length);
Copy link

@camilajenny camilajenny Jun 20, 2025

Choose a reason for hiding this comment

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

this is not a problem coincidentally because you have return 0, but if you do it correctly it will cause out of array error, replace it with int.min and do the length comparison as above

for (int i = 0; i < length; i++) {
int rc = strcmp (aparts[i], bparts[i]);
if (i == length - 1) {
if (bparts[i] > aparts[i]) {
return 1;
}
else if (bparts[i] < aparts[i]) {
int a_num, b_num;
bool a_is_num = int.try_parse(aparts[i], out a_num);
bool b_is_num = int.try_parse(bparts[i], out b_num);

if (a_is_num && b_is_num) {
if (a_num < b_num) {
Copy link

@camilajenny camilajenny Jun 20, 2025

Choose a reason for hiding this comment

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

if(a_num != b_num)
return a_num - b_num;

is also faster for cpu than comparisons

return -1;
}
}
if (rc < 0) {
return -1;
} else if (rc > 0) {
return 1;
} else if (a_num > b_num) {
return 1;
}
return 0;
Copy link

@camilajenny camilajenny Jun 20, 2025

Choose a reason for hiding this comment

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

for the strings "32.17.3" and "32.17.10" the method will return 0 and this is the culprit. Remove it and add a if(i == length-1) like in the original code

} else {
return strcmp (aparts[i], bparts[i]);
Copy link

@camilajenny camilajenny Jun 20, 2025

Choose a reason for hiding this comment

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

this changes the contract as strcmp doesn't have to return either -1, or 1 but any negative or positive number (cmp. below)

}
}

Expand Down Expand Up @@ -401,7 +401,7 @@ public class Eddy.Package : Object {
yield update_installed_state ();
return true;
}

} catch (Error e) {
throw e;
}
Expand All @@ -426,7 +426,7 @@ public class Eddy.Package : Object {
found = true;
state_flags = StateFlags.INSTALLED;

int rc = compare_versions (package.get_version (), version);
int rc = compare_versions (package.get_version (), version);
if (rc == 1) {
Copy link

@camilajenny camilajenny Jun 20, 2025

Choose a reason for hiding this comment

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

see above

state_flags |= StateFlags.CAN_DOWNGRADE;
} else if (rc == -1) {
Expand Down