Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented Mar 3, 2020

leveraging archspec if it's installed (https://github.com/archspec/archspec)

@boegel boegel added this to the next release (4.1.2?) milestone Mar 3, 2020
@easybuilders easybuilders deleted a comment from boegelbot Mar 7, 2020
@easybuilders easybuilders deleted a comment from boegelbot Mar 7, 2020
@migueldiascosta
Copy link
Member

worth also showing the arch name in eb --show-system-info, if it is known?

@boegel
Copy link
Member Author

boegel commented Mar 29, 2020

worth also showing the arch name in eb --show-system-info, if it is known?

Good point, fixed in 1d571f0, should be good to go now?

" -> family: %s" % get_cpu_family(),
]
if cpu_arch_name:
lines.append(" -> arch name: %s" % cpu_arch_name)
Copy link
Member

Choose a reason for hiding this comment

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

I think there is a small inconsistency... if here you mean exactly this, then without archspec it will still be printed, but UNKNOWN, and in test_show_system_info there is no reason to check for HAVE_ARCHSPEC, since the pattern will always be there

otherwise this should be if cpu_arch_name != UNKNOWN, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should hint that archspec is not installed when cpu_arch__name == UNKNOWN?

Copy link
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

lgtm

@migueldiascosta
Copy link
Member

Going in, thanks @boegel!

@migueldiascosta migueldiascosta merged commit 861ae81 into easybuilders:develop Mar 30, 2020
@boegel boegel deleted the cpu_arch_name branch March 30, 2020 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants