https://bugzilla.redhat.com/show_bug.cgi?id=1343977 Vít Ondruch <vondruch@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(vondruch@redhat.c | |om) | --- Comment #13 from Vít Ondruch <vondruch@xxxxxxxxxx> --- (In reply to Tummala Dhanvi (c0mrad3) from comment #12) > > > > > > > > * Package version > > > > - Please use proper version scheme. The package version should be probably > > > > just "0.1.0" while the release should be "0.1.dev". > > > > - The versioning is documented in detail here [2]. > > > I upstream had it versioning like that > > > https://github.com/asciidoctor/asciidoctor-mallard/blob/master/lib/ > > > asciidoctor-mallard/version.rb > > > > > > but I have updated it to 0.1.dev > > > > Please read the [2] carefully. You have to ensure update path. E.g. the > > original VR you used was "0.1.0.dev-1". If the upstream released stable > > version "0.1.0", your VR would become "0.1.0.dev-1" and this is what RPM > > thinks about the versions: > > > > ``` > > $ rpmdev-vercmp 0.1.0.dev-1 0.1.0-1 > > 0.1.0.dev-1 > 0.1.0-1 > > ``` > > > > That means the newer version would never get installed. Hence I suggested to > > use just "0.1.0" while the release should be "0.1.dev". In the .spec file, > > it would look like: > > > > ``` > > Version: 0.1.0 > > Release: 0.1.dev%{?dist} > > ``` > > > > Which makes the upgrade path correct: > > > > ``` > > $ rpmdev-vercmp 0.1.0-0.1.dev 0.1.0-1 > > 0.1.0-0.1.dev < 0.1.0-1 > > ``` > > > > Again, please read the [2] carefully (although I admit it is not easy read, > > but hopefully this will get simplified in the future). > Got it right this time! Yep, LGTM! But you still should work on the Source package I mentioned in previous step. Please study this [1] to get yourself familiar with git snapshots. This is something I come up with: ``` --- rubygem-asciidoctor-mallard.spec.orig 2016-12-05 14:57:16.684010918 +0100 +++ rubygem-asciidoctor-mallard.spec 2016-12-06 10:23:44.589045687 +0100 @@ -1,14 +1,26 @@ %global gem_name asciidoctor-mallard +%global commit0 cf5a2a8baf8f54f64af4d0f3eac763e1ca47c917 +%global shortcommit0 %(c=%{commit0}; echo ${c:0:7}) + +%global prever .dev +%global prerpmver %(echo "%{?prever}" | sed -e 's|\\.||g') + +# Modify the standard if we are packaging pre-release version. +%global gem_instdir %{gem_dir}/gems/%{gem_name}-%{version}%{?prever} +%global gem_cache %{gem_dir}/cache/%{gem_name}-%{version}%{?prever}.gem +%global gem_spec %{gem_dir}/specifications/%{gem_name}-%{version}%{?prever}.gemspec +%global gem_docdir %{gem_dir}/doc/%{gem_name}-%{version}%{?prever} + Name: rubygem-%{gem_name} Version: 0.1.0 -Release: 0.1.dev%{?dist} +Release: %{?prever:0.}1%{?prever:.%{prerpmver}}%{?dist} Summary: Converts AsciiDoc documents to Project Mallard format Group: Development/Languages License: MIT URL: https://github.com/asciidoctor/asciidoctor-mallard -Source0: %{gem_name}-%{version}.gem +Source0: https://github.com/asciidoctor/asciidoctor-mallard/archive/%{commit0}.tar.gz#/%{name}-%{shortcommit0}.tar.gz BuildArch: noarch BuildRequires: ruby @@ -30,17 +42,20 @@ Documentation for %{name} %prep -gem unpack %{SOURCE0} - -%setup -q -D -T -n %{gem_name}-%{version} +%setup -n %{gem_name}-%{commit0} -gem spec %{SOURCE0} -l --ruby > %{gem_name}.gemspec +# We don't have git repository, so just collect everything available, +# except the .gemspec (which is modified by this command and the gem itself. +sed -i '/s.files/a \ + s.files = Dir["**/*"] - ["asciidoctor-mallard-#{Asciidoctor::Mallard::VERSION}.gem", "asciidoctor-mallard.gemspec"]' \ + asciidoctor-mallard.gemspec %build # Create the gem as gem install only works on a gem file gem build %{gem_name}.gemspec -%gem_install +# NOTE: The '-n ...' can be dropped when stable version is available. +%gem_install -n %{gem_name}-%{version}%{?prever}.gem %install mkdir -p %{buildroot}%{gem_dir} @@ -93,8 +108,10 @@ %files doc %{gem_docdir} +%{gem_instdir}/Gemfile %{gem_instdir}/README.adoc %{gem_instdir}/Rakefile +%{gem_instdir}/WORKLOG.adoc %{gem_instdir}/test %changelog ``` > > > > > > * Test suite > > > > - Well, the line you used is just part of the story. You can see that there > > > > is nothing which would indicated, that the test suite was executed. You > > > > should use following line: > > > > > > > > ``` > > > > ruby -Ilib -e 'Dir.glob "./test/*_test.rb", &method(:require)' > > > > ``` > > > > > > > > - Unfortunately, there seems to be dependency on asciidoctor-doctest, which > > > > is not in Fedora yet. Since it has quite lot of dependencies, it is > > > > probably not worth of packaging ATM, but all this should be documented. > > > > - Instead of execution of test suite, you should consider to provide at > > > > least > > > > some sanity test, e.g. try to convert some document from AsciiDoc to > > > > Mallard using the %{gem_instdir}/bin/asciidoctor-mallard > > > > > > > Can you please explain me a bit more about how to get it right ? > > > > > > URL: https://dhanvi.fedorapeople.org/packages/asciidoctor-mallard/ > > > > My idea was to do something as simple as shipping some simple AsciiDoc file > > as Source1 and trying the conversion, e.g. the %check section could look > > like: > > > > ``` > > %check > > pushd .%{gem_instdir} > > # There is no rubygem-asciidoctor-doctest in Fedora yet, which is needed > > # for test suite. > > # ruby -Ilib -e 'Dir.glob "./test/*_test.rb", &method(:require)' > > > > bin/asciidoctor-mallard %{SOURCE1} -o test.output > > popd > > > > ``` > > > > This is the simplest test you can do, since if the conversion fails for > > whatever reason, the build should break. > > > > On top of that, you can check for the consistency of the output by > > calculating hash of the output, e.g. you can add the following line there: > > > > ``` > > sha256sum test.output | grep > > "870474333c6f4e41238f923f1d27c7688c86b6463db39725e076fc5f05fe2520" > > ``` > done it using the example given in the README > > > > Of course you have to use correct hash, based on the content of the > > %{SOURCE1} file you are going to use. Or you can choose to test any other > > bit of the file content, if you consider it important. > > > > > > BTW just running "bin/asciidoctor-mallard" will fail the build ATM, since > > there is apparently some bug (undefined method `size' for nil:NilClass). > > > This might be due to the requirements of rubygem-asciidoctor which I missed, > also > > ` > ruby ./bin/asciidoctor-mallard > ` > > works. Wonderful! Thx. * Please drop the "Requires: rubygem-asciidoctor". This require is autogenerated, you don't need to specify this dependency explicitly. [1] https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Hosting_Services -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx