Fedora support

[richardhenwood]

Rebase of @ycollet work, so it can merge.

[richardhenwood]

@falkTX : This is rebase of the ycollet patches for fedora support. If you choose to merge this (which would be great!) you can close #162 and #122 .

PS. I couldn't figure out a way to update #162 with this simple rebase, so I've issued a new pull request from my fork.

[falkTX]

The PR is clean enough, I would just adapt it a little bit for personal preference.

1. LIB_PATH should to be stored in a common place, so we can define it just once and import it where needed.
   to make it does not conflict with debian packages, I suggest having the check as this:

if os.path.isdir("/usr/lib64") and os.path.exists("/bin/rpm")
    LIBDIR = "/usr/lib64"
else:
    LIBDIR = "/usr/lib"

2. Using "string" + varname + "string" syntax is quite ugly, and does not fit the rest of the code.
   Use either "string %s %smore stuff" % (var1, var2) or "string stuff {} {} more stuff".format(var1, ...)"

[ycollet]

Thanks for the rebase ! I was about to do it when I saw that it has already be done.

…

----- Mail original ----- De: "richardhenwood" <notifications@github.com> À: "falkTX/Cadence" <Cadence@noreply.github.com> Cc: "ycollet" <ycollette.nospam@free.fr>, "Mention" <mention@noreply.github.com> Envoyé: Mardi 12 Février 2019 02:36:32 Objet: Re: [falkTX/Cadence] Fedora support (#244) @falkTX : This is rebase of the @ycollet patches for fedora support. Once this patch is merged, #162 and #122 can be closed as they are redundant. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub , or mute the thread .

[richardhenwood]

@falkTX : 1. I don't have a good understanding of the project architecture. It looks to me like lib_path is only used in claudia_launcher.py, and so from my naive perspective, the current self.lib_path definition seems 'good enough'. I am happy to make changes, I just need a bit more guidance.

2. Yes, I like the 'format' syntax. I've added a new commit new use that.

[falkTX]

previous code had the default value stored in a variable.
if you do that, you should no longer need to split the string into multiple lines

[falkTX]

Thanks for the commit-fix.
What I mean with the first point is that the check for "/usr/lib64" is done in 2 different places.
I would preferably do that in a single shared place instead.

[richardhenwood]

I've got a bit confused about where I am with this pull request after some time away. It looks like I've made the check global in 'shared_cadence.py'. @falkTX do you see this recent change - and do you have more feedback?

[richardhenwood]

oops. I see the current change doesn't work in practice. I'll fix and update this PR.

[richardhenwood]

Ok, I fixed a typo, and tested successfully: @falkTX any more suggestions?

[falkTX]

I would manually do some small things, but PR is okay as it is right now.
I am not just doing work in cadence right now. will come back to it later in the year.

[richardhenwood]

@falkTX is it late enough in this year to consider this merge? :)

[simotek]

The PR is clean enough, I would just adapt it a little bit for personal preference.

1. `LIB_PATH` should to be stored in a common place, so we can define it just once and import it where needed.
   to make it does not conflict with debian packages, I suggest having the check as this:

if os.path.isdir("/usr/lib64") and os.path.exists("/bin/rpm")
    LIBDIR = "/usr/lib64"
else:
    LIBDIR = "/usr/lib"

Technically I think this would probably break i586 / i686 and probably also 32bit arm

[simotek]

Carla suffers from the same issue, but i'll wait for you decide how to fix it here to submit something there.