Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#51344 closed defect (fixed)

seqan @2.1.1_0 Fixes installation of missing files

Reported by: rrahn Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 2.3.4
Keywords: haspatch maintainer Cc:
Port: seqan

Description

Adds two more commands in destroot script to:

  • install FindSeqan.cmake into ${destroot}${prefix}/share/$cmake, whereby $cmake is determined by a pre-destroot script, and
  • install seqan-2.pc into ${destroot}${prefix}/share/pkgconfig.

These files are needed, so other projects can build against the SeqAn library more easily.

Attachments (1)

Portfile-seqan.diff (903 bytes) - added by rrahn 7 years ago.
Cleaned up patch file

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by mf2k (Frank Schima)

Priority: HighNormal

The Priority field is for use by Macports team members only.

comment:2 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

Keywords: haspatch maintainer added; seqan removed

If this changes the files the port installs, the port's revision must be increased.

comment:3 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

It looks like the code in the pre-destroot block will cause the port to install files into different places, depending on what directories exist on the user's system. We don't want that; we want port installs to be reproducible and the same on every system.

On my system I see both a share/cmake and a share/cmake-3.5 directory? Do we know why this is?

comment:4 in reply to:  3 Changed 8 years ago by rrahn

Replying to ryandesign@…:

It looks like the code in the pre-destroot block will cause the port to install files into different places, depending on what directories exist on the user's system. We don't want that; we want port installs to be reproducible and the same on every system.

On my system I see both a share/cmake and a share/cmake-3.5 directory? Do we know why this is?


I am also not so happy with this solution, but needed a fallback option, if cmake is not installed yet, since we don't require it for our library installation. Another approach would be to always install into ${prefix}/share/cmake (which seems to be the default location on linux systems) and notify the user to set a symlink to this module from within the installed cmake module path? I just realised, that if cmake is installed via dmg installer the default modules location would be under package contents in the applications folder, wouldn't it? So this would solve the problem of variable install locations and different cmake installation directories.

But I am not so sure, what here the policies are with user messages during the installation phase?!

comment:5 Changed 7 years ago by rrahn

Hey, I updated the portfile. The patch will now install all files to a fixed location.

comment:6 Changed 7 years ago by raimue (Rainer Müller)

The patch file is somewhat garbled, as it contains a unified diff followed by two diffs in normal format. These diffs apply different changes and I cannot deduce what your intention was.

When changing files that will be installed, you will also need to increase the revision:

  • science/seqan/Portfile

    a b PortSystem 1.0 
    55
    66name                seqan
    77version             2.1.1
     8revision            1
    89categories          science
    910platforms           darwin
    1011supported_archs     noarch

comment:7 Changed 7 years ago by rrahn

Oh, sorry. I'll fix that and also increase the revision number. Thx for pointing this out.

comment:8 Changed 7 years ago by rrahn

Ok, I hope the file os now ok. Thanks again.

comment:9 Changed 7 years ago by raimue (Rainer Müller)

The preferred format for diffs would be unified, which is usually created by running diff -u Portfile.orig Portfile as described in the guide.

Are you sure these are the right changes? The directory structure you are creating does not make sense.

  • The previous copy statement for include looked fine. What is the addition of a trailing slash supposed to solve?
  • This now creates ${prefix}/share/doc/doc/seqan/ (duplicate doc/)
  • The .pc file needs to go to ${prefix}/lib/pkgconfig/. Similarly for the .cmake file, which would be expected in ${prefix}/lib/cmake/${name}/. What is the point in not putting them into the default search paths?

comment:10 Changed 7 years ago by rrahn

Right, these first two path changes were not intentionally. For the pkgconfig, I wasn't actually sure but thanks for pointing that out. It seems that ${prefix}/share/pkgconfig is also a valid path. For cmake the given path is in the standard search path according to the find_package documentation. But if you prefer to install it into lib I can do that.

Changed 7 years ago by rrahn

Attachment: Portfile-seqan.diff added

Cleaned up patch file

comment:11 Changed 7 years ago by raimue (Rainer Müller)

Yes, both ${prefix}/lib/pkgconfig and ${prefix}/share/pkgconfig would be valid. I don't think there is any strong preference, but the former appears to be more popular among ports.

I am no cmake expert and I don't know if there is any preference or difference in the Find*.cmake or *-config.cmake files. I am just wondering why upstream decides to use the former and then it is changed here to the latter.

comment:12 Changed 7 years ago by rrahn

Actually, FindSeqAn will be renamed to seqan-config in the upcoming release, as it is the proper way of distributing the package info for cmake. For 2.1.1 and 2.2.0 we have to manually rename it during installation process. I updated the port file to install now into ${prefix}/lib/pkgconfig and ${prefix}/lib/cmake/seqan`. I hope I got it right this time.

Many thanks for the support.

comment:13 Changed 7 years ago by raimue (Rainer Müller)

Resolution: fixed
Status: newclosed

In 8289af5/macports-ports:

seqan: fix installation of missing files

Closes: #51344 (maintainer)

comment:14 Changed 7 years ago by raimue (Rainer Müller)

In 7a94950/macports-ports:

seqan: fix installation of missing files

Closes: #51344 (maintainer)

Note: See TracTickets for help on using tickets.