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

2 empty lines geneated in a template #104

Open
junaruga opened this issue Jan 7, 2021 · 3 comments
Open

2 empty lines geneated in a template #104

junaruga opened this issue Jan 7, 2021 · 3 comments

Comments

@junaruga
Copy link
Contributor

junaruga commented Jan 7, 2021

The current gem2rpm creates 2 empty lines between cp -a and mkdir -p for a generated spec file.

As a result on the following PR line 13, the 2 lines is changed to 1 line.
https://src.fedoraproject.org/rpms/rubygem-rack/pull-request/3#request_diff

1 empty line is better for the template than 2 empty lines, isn't it?

Here is the log for rubygem-rack.

$ which gem2rpm
/usr/bin/gem2rpm

$ rpm -qf /usr/bin/gem2rpm
rubygem-gem2rpm-1.0.1-6.fc33.noarch

$ gem2rpm rack-2.2.3.gem > rubygem-rack.spec.new

$ cat rubygem-rack.spec.new
...
%install
mkdir -p %{buildroot}%{gem_dir}
cp -a .%{gem_dir}/* \
        %{buildroot}%{gem_dir}/


mkdir -p %{buildroot}%{_bindir}
cp -a .%{_bindir}/* \
        %{buildroot}%{_bindir}/
...
@voxik
Copy link
Member

voxik commented Jan 7, 2021

  1. I don't personally mind the spaces. Admittedly, in this specific case, it would be probably better if they are gone.
  2. I am afraid this is not as easy to solve. If you check the template, there are not additional empty lines spaces:
    %install
    mkdir -p %{buildroot}%{gem_dir}
    cp -a .%{gem_dir}/* \
    %{buildroot}%{gem_dir}/
    <% unless spec.extensions.empty? -%>
    mkdir -p %{buildroot}%{gem_extdir_mri}
    cp -a .%{gem_extdir_mri}/{gem.build_complete,*.so} %{buildroot}%{gem_extdir_mri}/
    <% for ext in spec.extensions -%>
    # Prevent dangling symlink in -debuginfo (rhbz#878863).
    rm -rf %{buildroot}%{gem_instdir}/<%= ext.split(File::SEPARATOR).first %>/
    <% end -%>
    <% end -%>
    <% unless spec.executables.empty? -%>
    mkdir -p %{buildroot}%{_bindir}
    cp -a .%{_bindir}/* \
    %{buildroot}%{_bindir}/

    The problem is that the template tags are expanded into empty lines and AFAIK, using ERB, there is no way to get easily rid of them. In theory they could be remove for the price of worse template readability, what is not win either.

@pvalena
Copy link
Contributor

pvalena commented Jan 7, 2021

You're right, if we stick to this formatting.

The lines 60 and 70 are responsible for the double space. Could be resolved by writing it like so. f.e.:

mkdir -p %{buildroot}%{gem_dir}
cp -a .%{gem_dir}/* \
        %{buildroot}%{gem_dir}/
<%
unless spec.extensions.empty? -%>

Or any other adjustment which changes one of those empty lines to either end with -%> or start with <%.

@junaruga
Copy link
Contributor Author

junaruga commented Jan 7, 2021

The problem is that the template tags are expanded into empty lines and AFAIK, using ERB, there is no way to get easily rid of them. In theory they could be remove for the price of worse template readability, what is not win either.

I do not think so. You can see https://puppet.com/docs/puppet/5.5/lang_template_erb.html . -%> trims 1 line break after -%>. I see the reason is simply how to write the template is not correct. I sent a PR.

-%> — If the tag ends a line, trim the following line break.

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 a pull request may close this issue.

3 participants