Add warning to deprecated macros

rvais commented 8 years ago

Solves #20 for this repository. I added warning message via 'echo' command to deprecated macros. changing documentation will follow.

rvais commented 8 years ago

I editied documentation in my fedora-java/howto fork to new branche. I couldn't find any documentation there for %ant and %add_maven_depmap is already documented as depricated so %jpackage_script is the only macro I changed in documentation ... Please review it as well and add some comment.

mizdebsk commented 8 years ago

Looks good, but please revert %ant deprecation as discussed in #20

mizdebsk commented 8 years ago

Actually I think it would be better if warnings were printed on rpm stderr, like:

%somemacro %{lua:io.stderr:write("[WARNING] Deprecated %somemacro macro is used.\\n")}OriginalMacroDefinition

This way warning is printed early, for example when creating SRPM. Even rpmlint produces error, so that package which uses deprecated macros should not pass review:

foo.spec: E: specfile-error [WARNING] Deprecated %somemacro macro is used.
0 packages and 1 specfiles checked; 1 errors, 0 warnings.

rvais commented 8 years ago

Ok, sure. I will change that ...

codecov-io commented 8 years ago

Current coverage is 87.76% (diff: 100%)

Merging #24 into master will not change coverage

@@             master        #24   diff @@
==========================================
  Files            44         44          
  Lines          3156       3156          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           2770       2770          
  Misses          386        386          
  Partials          0          0

Powered by Codecov. Last update ccd4f9b...a320441

rvais commented 8 years ago

I changed echo for lua:io and squished all commits again. Please review ...

mizdebsk commented 8 years ago

Did you test these warnings at all? They don't seem to work as expected.

For example, I get that:

Start: build setup for ant-antunit-1.3-4.fc25.src.rpm
[WARNING] Deprecated %%add_maven_depmap macro is used.ntThis macro has been made obsolete.ntPlease use %%mvn_artifact() instead.nwarning: Could not canonicalize hostname: mizdebsk.usersys.redhat.com
Building target platforms: x86_64

mizdebsk commented 8 years ago

:+1: Looks good. I will wait with merging it until launcher is implemented.

rvais commented 8 years ago

It might look good, but it causes builds to fail even if they do not use these macros ... I do not think it is supposed to do that. I can't figure out the reason for that. I was using jcommander package to test that. It does not use either deprecated macro, but build ends with error. I wasn't able to find anything that would tell me what is wrong in logs.

mizdebsk commented 8 years ago

@rvais I'll test it on beust-jcommander package.

mizdebsk commented 8 years ago

Works for me - beust-jcommander package build fine with changes of this PR applied.

This is basically what I did to test it:

# Build pull request
git checkout -b test-deprecation master && git merge rvais/DepricatedMacros
./configure --prefix=/usr && ./build

# Install built macros in mock chroot
mock --init && mock -i maven-local
mock --copyin target/macros.{fjava,jpackage} /usr/lib/rpm/macros.d/

# Rebuild beust-jcommander package
mock --no-clean --buildsrpm --spec *.spec --sources . --resultdir .
mock --no-clean *.src.rpm

mizdebsk commented 7 years ago

This has been resolved in commit 2850f999a08e648ffec7f8d0b914333452033d99. Closing this PR.

fedora-java / javapackages

Add warning to deprecated macros #24

Current coverage is 87.76% (diff: 100%)