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 the ability to use concat for zshrc #31

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

Conversation

Safranil
Copy link

This will add the ability to give a preconfigured ZSH for the user using fragment instead of a full flat file.

If concat is used, a second file is added in the user directory named .zshrc.local. This file allow the user to customize his zshrc without losing changes.

This allow the configuration of Powerline instant prompt as it need to be on top of the zshrc file

Exemple:

class { 'ohmyzsh':
    concat => true,
}

concat::fragment { '/root/.zshrc:p10k-instant-prompt':
    target => '/root/.zshrc',
    source => "puppet:///modules/${module_name}/p10k-instant-prompt.zshrc",
    order  => '005',
}

concat::fragment { '/root/.zshrc:p10k-load':
    target  => '/root/.zshrc',
    content => "[[ ! -f ~/.p10k.zsh ]] || source ~/.p10k.zsh\n",
    order   => '095',
}

@rehanone
Copy link
Owner

Thanks for creating the pr. can you please rebase it to master. I will review it as soon as I have some free time.

@Safranil
Copy link
Author

I rebased my commit for the change involved in #34.

@Safranil
Copy link
Author

I rebased the branch to get the changes from e57a0f9.

I moved the default value of ohmyzsh::concat from init.pp to data/common.yaml.

@rehanone
Copy link
Owner

@Safranil , looks like some tests are failing, can you check your PR please?

@rehanone
Copy link
Owner

I am not familiar with .zshrc.local . Can you link some documentation to this please?

@Safranil
Copy link
Author

Safranil commented Nov 21, 2022

I am not familiar with .zshrc.local . Can you link some documentation to this please?

It's specific to this module, with this file you can customize your ZSH while Puppet is managing the .zshrc file.
Puppet will create this file for you and your local changes (e.g. aliases) can be added to this file.

I added this for some use case by a customer of me.

@Safranil
Copy link
Author

Puppet 6 on Ubuntu 20.04

I don't really understand why the build is not working... The tests commands doesn't seem to be the same on my PR and on master.

Master : https://app.travis-ci.com/github/rehanone/puppet-ohmyzsh/jobs/588776307#L697
PR : https://app.travis-ci.com/github/rehanone/puppet-ohmyzsh/jobs/589028645#L707

On master and PR the command wget -O /tmp/puppet.deb https://apt.puppet.com/puppet6-release-focal.deb are the same but the next command dpkg -i --force-all /tmp/puppet.deb don't give the same result:

On master the log is (L717):

  Configuration file '/etc/apt/sources.list.d/puppet6.list', does not exist on system.
  Configuration file '/etc/apt/trusted.gpg.d/puppet6-keyring.gpg', does not exist on system.

But on my PR the log is (L727):

  Configuration file '/etc/apt/sources.list.d/puppet6-stable.list', does not exist on system.
  Configuration file '/etc/apt/trusted.gpg.d/puppet6-stable-keyring.gpg', does not exist on system.

And finally apt-get update give an error on my PR (L735):

  Err:3 https://apt.repos.puppet.com/puppet6 focal Release
    Certificate verification failed: The certificate is NOT trusted. The certificate chain uses expired certificate.  Could not handshake: Error in the certificate verification. [IP: 34.149.154.163 443]

My PR go on apt.repos.puppet.com but master is going on apt.puppetlabs.com (L763), an issue is open in Puppet GitHub Project and Jira.

I don't know where is set this repository.

Puppet 6 on Debian 8

I don't know what is going wrong with my PR.

@rehanone
Copy link
Owner

Looks like the tests are falling for some expired cert. I will check if I can fix it. Also, I like the idea of having a .local file with customizations but i think it should start simple. perhaps just one concat file to start off. can you please simplify this PR and split it so it is easier to see all the changes?

@Safranil
Copy link
Author

Safranil commented Dec 3, 2022

Looks like the tests are falling for some expired cert. I will check if I can fix it. Also, I like the idea of having a .local file with customizations but i think it should start simple. perhaps just one concat file to start off. can you please simplify this PR and split it so it is easier to see all the changes?

I have done it, I removed the local part and focused the PR on the concat functionality.

Edit: I see the build is now passing for Puppet 6 on Ubuntu 20.04, but the same problem as before occur on Debian 8.

@rehanone
Copy link
Owner

rehanone commented Dec 3, 2022

I have removed debian 8 from tests. It looks like that image is not on dockerhub anymore. Try to rebase now.

@Safranil
Copy link
Author

Safranil commented Dec 3, 2022

I have removed debian 8 from tests. It looks like that image is not on dockerhub anymore. Try to rebase now.

Done.

@Safranil Safranil force-pushed the feature/concat branch 3 times, most recently from 2484725 to ea0dafd Compare April 20, 2023 09:44
@Safranil
Copy link
Author

I simplified my PR to just include a base for working with the concat module.

I also split the concat and the classic method in two sub resources for a better readability.

I see the CI fail on the documentation on files without any docs prior my PR, I will do a second PR to add the missing documentation.

@Safranil Safranil mentioned this pull request Apr 20, 2023
@Safranil Safranil force-pushed the feature/concat branch 2 times, most recently from 4a01f8c to 69f1065 Compare April 20, 2023 11:39
@Safranil Safranil marked this pull request as draft April 20, 2023 13:02
This will add the ability to give a preconfigured ZSH for the user and
allow the configuration of Powerline instant prompt as it need to be on
top of the zshrc file

This commit also include an example code for this case
This will add the ability to give a preconfigured ZSH for the user and
allow the configuration of Powerline instant prompt as it need to be on
top of the zshrc file

This commit also include an example code for this case
@Safranil Safranil marked this pull request as ready for review April 20, 2023 13:44
@Safranil
Copy link
Author

@rehanone can you retry the failed build step, a timeout occurred.

This PR also contains changes involved in #41 as it was simpler to see the errors from the first build step.

@Safranil
Copy link
Author

Safranil commented Jun 3, 2024

@rehanone, can you tell me if this changes will be merged in the module?

Allow concat 8.x and 9.x
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