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

add script for symlinking paths in $EPREFIX to corresponding paths in host root #34

Merged
merged 3 commits into from
Sep 22, 2020

Conversation

boegel
Copy link
Contributor

@boegel boegel commented Sep 21, 2020

cfr. #15

Copy link
Collaborator

@bedroge bedroge left a comment

Choose a reason for hiding this comment

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

Looks okay to me, though I was working on something similar with Ansible and I would prefer to have it there: I think that's cleaner, and it keeps all the configuration for the compatibility layer in a single file.

ec=$?
if [ $ec -ne 0 ]; then
echo_yellow ">> [CHANGE] ${EPREFIX}$path is *not* a symlink to $path, fixing that..."
rm ${EPREFIX}$path
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we want to do the same for directories (as CC suggested), this will have to be a bit more aggressive ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but since it's only files right now, I'd keep it at that for now.

@boegel
Copy link
Contributor Author

boegel commented Sep 21, 2020

Looks okay to me, though I was working on something similar with Ansible and I would prefer to have it there: I think that's cleaner, and it keeps all the configuration for the compatibility layer in a single file.

That's not entirely true, since the set of packages to install is already in a separate file too...

There's value in being able to run the steps "manually" as well outside of Ansible imho?

@bedroge
Copy link
Collaborator

bedroge commented Sep 21, 2020

True, but that file had to be part of the overlay repo. If I would use the Ansible playbook for installing the compatibility layer, I would prefer to have it all together (as much as possible), instead of having to edit both my variables file and bash scripts. I do see the added value of being able to run things manually, though. And since we already describe a manual procedure in the README anyway, maybe we should split it all up and make two different dirs with Ansible and bash stuff?

@boegel
Copy link
Contributor Author

boegel commented Sep 21, 2020

@bedroge Splitting things up makes sense to me...

Copy link
Collaborator

@bedroge bedroge left a comment

Choose a reason for hiding this comment

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

Could you also update .github/workflows/ansible-lint.yml and change line 26:

        targets: "playbooks/*.yml"

to:

        targets: "ansible/playbooks/*.yml"

@boegel
Copy link
Contributor Author

boegel commented Sep 22, 2020

@bedroge Nice catch... Done in 0a5f04b

Copy link
Collaborator

@bedroge bedroge left a comment

Choose a reason for hiding this comment

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

lgtm

@bedroge bedroge merged commit 4f57d60 into EESSI:master Sep 22, 2020
@boegel boegel deleted the prefix_symlink_paths branch September 22, 2020 12:21
poksumdo pushed a commit to poksumdo/compatibility-layer that referenced this pull request Jun 8, 2023
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.

2 participants