added fix for findutilsport

[netskink]

This is a fix for this bug #567

It appears the original code was attempting to strip strings but it did not work. Here is some explantion.

# You can use # as a delimeter, rather than /
# ie. s/xx/yy/ is same as s#xx#yy#
# double // is a cut

echo ${test_var} | sed -e s#=.*##
# returns:
# findutilsport


echo ${test_var} | sed -e s#.*port##
# returns:
# =12%4

echo ${test_var} | sed -e s#port##
# returns:
# findutils=12%4

echo ${test_var} | sed -e s#%.*##
# returns:
# findutilsport=12

@MikeFultonDev @DevonianTeuchter

[MikeFultonDev]

This might be a difference between /bin/sed and our sed so to be safe you may want to also say /bin/sed ?

[netskink]

This might be a difference between /bin/sed and our sed so to be safe you may want to also say /bin/sed ?

Hmm, I was just making a change to all the sed occurances in the file and going to make it ~/zopen/usr/local/bin/sed instead. This kind of distracts and not ideal. Then I thought I should rather do something like get the location of the script and set the path to be relative to there.

ie hermetic build thing.

Then I thought I should clarify with you, how do you want to do that? Or, perhaps do in a follow up issue. ie. this patch fixes the zopen install findutilsport problem where the strip port does nto work. Then do a separate patch to ensure sed is from zopen rather than USS default sed.

If I file as an issue, I can always revisit. I'm kind of thinking gnport is higher priority.

[DevonianTeuchter]

The code here is meant to do an up-front check to make sure that the port prefix (whether followed by a version, tag or "port") is valid. Further processing within handlePackageInstall uses that additional information for searching for versioned/tagged releases so stripping that now [ie passing toolrepo] will break that processing later.
The actual fix needs to be in handlePackageInstall()
name=$(echo "${fullname}" | sed -e 's#[=%].*##')
repo="${name}" <- this should have the "${name%port}" expansion
The twin parsing is so that the list of supplied packages can be parsed first; if the check is only done in the second instance, then you might have a number of packages specified and it would only be after installing all the previously "well specified" packages that you'd find the broken one.

The sed should probably use the zossed function [which maps to /bin/sed] - we don't want to hardcode the use of any zopen "sed" as that might not be installed...

[IgorTodorovskiIBM]

This might be a difference between /bin/sed and our sed so to be safe you may want to also say /bin/sed ?

Hmm, I was just making a change to all the sed occurances in the file and going to make it ~/zopen/usr/local/bin/sed instead. This kind of distracts and not ideal. Then I thought I should rather do something like get the location of the script and set the path to be relative to there.

ie hermetic build thing.

Then I thought I should clarify with you, how do you want to do that? Or, perhaps do in a follow up issue. ie. this patch fixes the zopen install findutilsport problem where the strip port does nto work. Then do a separate patch to ensure sed is from zopen rather than USS default sed.

If I file as an issue, I can always revisit. I'm kind of thinking gnport is higher priority.

Now that we rely on the zopen_releases.json cache (which doesn't have a port) suffix, the code to handle projects ending with port was removed. I'm fine with adding that support back in.

Do you get different results with zopen sed vs /bin/sed? The regex doesn't seem overly complex here so I doubt it will differ. We would we need to embed sed as we do with curl and jq if we wanted to go this route.

[netskink]

fwiw, this is the code in zopen-install as it originally was:

 printVerbose "Stripping any version (%), tag (#) or port suffixes"
    toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#.*port##')
    # sed can use # as a delimter rather than normal / delimiter.
    toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#port##')

and then later this in there

 # why strip if going to add as is anyway?
      # ORIG installArray="${installArray} ${chosenRepo}"
      installArray="${installArray} ${toolfound}"

I don't know all the variants for testing installs. (Do we have a unit test for install options?). So I was conjecturing in the comments above. The second test in the original comment shows that the .*port is not stripping "port", but instead stripping all text ending in port. Consquently, why I simply removed the match all bit.

In the second stanza, it seems its another bug. Its "stripping" text, but then the installarray adds the original non stripped entry. I was thinking the point of this code is to strip info and then add the stripped version to the list of repo's to install.

Based upon the comments I'm doubly confused now.

• Is this dead code which should be removed and the place to make the mod is in a different file and it has differernt logic?

• What are the various input forms that are possible with zopen install?

  • zopen install foo <- the only one I knnow which is valid
  • zopen install fooport <- realize the repo name is not how the install mechanism works and strip automatically to:
    • zopen install foo
  • zopen install foo=1.0 <- install a specific version of foo. perhaps this code adds to install array foo, but maninpulates aversion array somewhere?
  • `zopen install foo%sometag <- ditto?
  • are combos possible?
    *. zopen install fooport=1.0%sometag

[DevonianTeuchter]

fwiw, this is the code in zopen-install as it originally was:

 printVerbose "Stripping any version (%), tag (#) or port suffixes"
    toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#.*port##')
    # sed can use # as a delimter rather than normal / delimiter.
    toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#port##')

and then later this in there

 # why strip if going to add as is anyway?
      # ORIG installArray="${installArray} ${chosenRepo}"
      installArray="${installArray} ${toolfound}"

I don't know all the variants for testing installs. (Do we have a unit test for install options?). So I was conjecturing in the comments above. The second test in the original comment shows that the .*port is not stripping "port", but instead stripping all text ending in port. Consquently, why I simply removed the match all bit.

In the second stanza, it seems its another bug. Its "stripping" text, but then the installarray adds the original non stripped entry. I was thinking the point of this code is to strip info and then add the stripped version to the list of repo's to install.

Based upon the comments I'm doubly confused now.

• Is this dead code which should be removed and the place to make the mod is in a different file and it has differernt logic?

• What are the various input forms that are possible with zopen install?

  • zopen install foo <- the only one I knnow which is valid

  • zopen install fooport <- realize the repo name is not how the install mechanism works and strip automatically to:

    • zopen install foo

  • zopen install foo=1.0 <- install a specific version of foo. perhaps this code adds to install array foo, but maninpulates aversion array somewhere?

  • `zopen install foo%sometag <- ditto?

  • are combos possible?
    *. zopen install fooport=1.0%sometag

Nope, not dead code - it's an initial sanity check that what was specified on the command line correlates to a valid package name before handing it off to more specialized processing later
As your examples show, specifying on the cli to install findutil, findutilport, findutil%atag or findutil=v.r.m.f should [currently*] all be valid, with the code you identified supposed to strip it back to "findutil" as an initial sanity check that the package specified is actually valid. Inside handlePackageInstall, the logic for actually handling tagged and versioned information needs the suffix still, hence why the "unstripped" version is passed through. However... the initial code that you fixed for the sed line does [currently*] need fixed as it matches and strips everything with the expression s#.*port## - but using the suggested s#port## might replace the first instance of port it finds; if ever there is a package such as "exporterport", that'll end up looking like "exterport"; that sed expression should really be s#port$##, s'\(.*\)port$#\1# or equivalent shell expansion or... with the addition of a -n $toolfound -type check as well in case of specifying zopen install port or zopen install =1.2.3.4 for example.

*That being said... as @IgorTodorovskiIBM mentions earlier, the port suffix was dropped from the actual naming of a package so we shouldn't really need to do any stripping of it these days [the version/tag will still be needed at this point]. If a user specifies 'pkgport', it should get rejected as an invalid package, with 'pkg' being the required install parameters. It would solve any issues if ever there was package called something like "export" or "thisport" - so just treat the input as the proper package and not worry about 'port' suffixes.

And as for unit testing, there is a framework that Mike put together that we are adding to, but a lot of testing of this type of thing is manual at the moment.

[hopefully that makes any sense!]

[netskink]

Nope, not dead code - it's an initial sanity check that what was specified on the command line correlates to a valid package name before handing it off to more specialized processing later As your examples show, specifying on the cli to install findutil, findutilport, findutil%atag or findutil=v.r.m.f should [currently*] all be valid, with the code you identified supposed to strip it back to "findutil" as an initial sanity check that the package specified is actually valid. Inside

The santity check before continuing makes sense. That is why you strip, verify the strip is in the list, but then add back
the non stripped to the list of install packages.

handlePackageInstall, the logic for actually handling tagged and versioned information needs the suffix still, hence why the "unstripped" version is passed through. However... the initial code that you fixed for the sed line does [currently*] need fixed as it matches and strips everything with the expression s#.*port## - but using the suggested s#port## might replace the first instance of port it finds; if ever there is a package such as "exporterport", that'll end up looking like "exterport"; that sed expression should really be s#port$##, s'\(.*\)port$#\1#

That is a good point. I'll update the diff to reflect that port could be anywhere in the string and only remove the last entry.

or equivalent shell expansion or... with the addition of a -n $toolfound -type check as well in case of specifying zopen install port or zopen install =1.2.3.4 for example.

This I'm unclear on. Possibly I understand some portion of it. If a if [ -n $toolfound ] its a nonzero string. You want to check for some particular value? Compare against what? The list of known repos?

*That being said... as @IgorTodorovskiIBM mentions earlier, the port suffix was dropped from the actual naming of a package so we shouldn't really need to do any stripping of it these days [the version/tag will still be needed at this point]. If a user specifies 'pkgport', it should get rejected as an invalid package, with 'pkg' being the required install parameters. It would solve any issues if ever there was package called something like "export" or "thisport" - so just treat the input as the proper package and not worry about 'port' suffixes.

Ok, so in this case, the conclusion is don't worry about stripping, simply look for the name and if its not a proper repo name, tell them during the sanity check. ie.

zopen install findutilsport

return

package name not found, despite that being the actual name of the repo.

And as for unit testing, there is a framework that Mike put together that we are adding to, but a lot of testing of this type of thing is manual at the moment.

Having tests would be a great help in this situation.

[hopefully that makes any sense!]

Yes, DT, it helps.

[MikeFultonDev]

We are taking a just in time approach here to improve our quality and testing so when we add new code or fix bugs please add a new test or two.
the testing framework is pretty simple - you add a test to the tests directory that exits with 0 if it works or non zero if it fails.
when I am back give me a shout if you aren’t sure or ask @IgorTodorovskiIBM - he has written at least one

[netskink]

sounds great I’ll work on that with Igor tomorrow

[IgorTodorovskiIBM]

Here's a simple one for update-cacert: https://github.com/ZOSOpenTools/metaport/pull/16/files. @DevonianTeuchter has added a couple as well recently

[netskink]

Thanks. I’ll use that as a guide.

[netskink]

/run tests

[IgorTodorovskiIBM]

Suggested change

	echo "Usage error: please install using base name without port suffix."
	echo "Usage error: please install using base project name without port suffix."

[netskink]

Will do tomorrow

[netskink]

done

[IgorTodorovskiIBM]

LGTM

[IgorTodorovskiIBM]

@DevonianTeuchter @MikeFultonDev are you ok with this?

@netskink looks like there's a conflict in /bin/zopen-install

[netskink]

I’ll address on Monday. Thanks for the update

[MikeFultonDev]

LGTM

[IgorTodorovskiIBM]

@netskink , regarding @DevonianTeuchter's comment here:

The code here is meant to do an up-front check to make sure that the port prefix (whether followed by a version, tag or "port") is valid. Further processing within handlePackageInstall uses that additional information for searching for versioned/tagged releases so stripping that now [ie passing toolrepo] will break that processing later. The actual fix needs to be in handlePackageInstall() name=$(echo "${fullname}" | sed -e 's#[=%].*##') repo="${name}" <- this should have the "${name%port}" expansion The twin parsing is so that the list of supplied packages can be parsed first; if the check is only done in the second instance, then you might have a number of packages specified and it would only be after installing all the previously "well specified" packages that you'd find the broken one.

I think what we want here is an upfront error when installing multiple ports. So for example:

zopen install git findutilsport

Will it install git first and then print the about findutilsport having the port suffix?

[netskink]

@netskink , regarding @DevonianTeuchter's comment here:

The code here is meant to do an up-front check to make sure that the port prefix (whether followed by a version, tag or "port") is valid. Further processing within handlePackageInstall uses that additional information for searching for versioned/tagged releases so stripping that now [ie passing toolrepo] will break that processing later. The actual fix needs to be in handlePackageInstall() name=$(echo "${fullname}" | sed -e 's#[=%].*##') repo="${name}" <- this should have the "${name%port}" expansion The twin parsing is so that the list of supplied packages can be parsed first; if the check is only done in the second instance, then you might have a number of packages specified and it would only be after installing all the previously "well specified" packages that you'd find the broken one.

I think what we want here is an upfront error when installing multiple ports. So for example:

zopen install git findutilsport

Will it install git first and then print the about findutilsport having the port suffix?

I don't know if it will handle multiple install packages. To be honest I never tired it. I just tried to do things like this:

$ zopen install foo
$ zopen install fooport

The original code never seemed to match. The subsquent comparison clause was never hit. ie if x==y where y was the result of the sed, was always blank for y.

FWIW, I removed the code in the clause and moved it to the top of the file since you said the first check was a sanity check. What remains at the bottom was based upon your request to collect the matching group.

I can test to see what multiple install packages do, but to be honest I believe this change is way over due and beyond the original objective. I propose we merge this one and raise a new issue. Fix one issue at a time and move on. I'm all for collaboration and am happy to adjust accordingly. Please don't let my ability to talk be taken negative. I just want to drive this to completion.

[netskink]

exercise with code in patch pull request present

Using the patch plus some debug code:

   for chosenRepo in $(echo "${chosenRepos}" | tr ',' '\n'); do
     printVerbose "Processing repo: ${chosenrepo}"
     printVerbose "Stripping any version (%), tag (#) or port suffixes"
+    echo  "chosenRepo: $chosenRepo"
     toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#\(.*\)port$#\1#')
+    #toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#.*port$##')
+    echo  "toolrepo: $toolrepo"
     printVerbose "Adding prefix and 'port' suffix" # quicker than testing for presence!
     toolfound=$(echo "${repo_results}" | awk -vtoolrepo="${toolrepo}" '$0 == toolrepo {print}')
     if [ "${toolfound}" = "${toolrepo}" ]; then
+      # also that is an = above and not a ==. I'm not sure what the intention is
+      echo "hitting the toolfound vs toolrepo check"
       printVerbose "Adding '${chosenRepo}' to the install queue"
       installArray="${installArray} ${chosenRepo}"
       printVerbose "Removing valid port from input list"

Tests

Try with port suffix added

$ zopen install findutilsport
- Querying remote repo for latest package information
chosenRepo: findutilsport
toolrepo: findutils
hitting the toolfound vs toolrepo check
Installing package: findutilsport
Usage error: please install using base project name without port suffix.
  try: zopen install findutils

Try with just repo name specified

$ zopen install findutils
- Querying remote repo for latest package information
chosenRepo: findutils
toolrepo: findutils
hitting the toolfound vs toolrepo check
Installing package: findutils
- Replacing findutils version '4.9.0.20231109_042435' with '4.9.0.20231115_215243'
- Found existing file 'findutils-4.9.0.20231115_215243.zos.pax.Z' in zopen package cache
- Processing findutils-4.9.0.20231115_215243.zos.pax.Z...
After this operation, 5.14 MB of additional disk space will be used.
Do you want to continue? [y/n]

Trying with some unknown repo and port suffix

$ zopen install fooport
- Querying remote repo for latest package information
chosenRepo: fooport
toolrepo: foo
***ERROR: The following requested port(s) do not exist:
	fooport

Trying with some unknown repo

$ zopen install foo
- Querying remote repo for latest package information
chosenRepo: foo
toolrepo: foo
***ERROR: The following requested port(s) do not exist:
	foo

exercise without patch present

   for chosenRepo in $(echo "${chosenRepos}" | tr ',' '\n'); do
     printVerbose "Processing repo: ${chosenrepo}"
     printVerbose "Stripping any version (%), tag (#) or port suffixes"
-    toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#\(.*\)port$#\1#')
+    echo  "chosenRepo: $chosenRepo"
+    #toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#\(.*\)port$#\1#')
+    toolrepo=$(echo "${chosenRepo}" | sed -e 's#%.*##' -e 's#=.*##' -e 's#.*port$##')
+    echo  "toolrepo: $toolrepo"
     printVerbose "Adding prefix and 'port' suffix" # quicker than testing for presence!
     toolfound=$(echo "${repo_results}" | awk -vtoolrepo="${toolrepo}" '$0 == toolrepo {print}')
     if [ "${toolfound}" = "${toolrepo}" ]; then
+      # also that is an = above and not a ==. I'm not sure what the intention is
+      echo "hitting the toolfound vs toolrepo check"
       printVerbose "Adding '${chosenRepo}' to the install queue"
       installArray="${installArray} ${chosenRepo}"
       printVerbose "Removing valid port from input list"

Tests

Try with port suffix added

$ zopen install findutilsport
- Querying remote repo for latest package information
chosenRepo: findutilsport
toolrepo:
hitting the toolfound vs toolrepo check
Installing package: findutilsport
Usage error: please install using base project name without port suffix.
  try: zopen install findutils

Try with just repo name specified

$ zopen install findutils
- Querying remote repo for latest package information
chosenRepo: findutils
toolrepo: findutils
hitting the toolfound vs toolrepo check
Installing package: findutils
- Replacing findutils version '4.9.0.20231109_042435' with '4.9.0.20231115_215243'
- Found existing file 'findutils-4.9.0.20231115_215243.zos.pax.Z' in zopen package cache
- Processing findutils-4.9.0.20231115_215243.zos.pax.Z...
After this operation, 5.14 MB of additional disk space will be used.
Do you want to continue? [y/n]

Trying with some unknown repo and port suffix

$ zopen install fooport
- Querying remote repo for latest package information
chosenRepo: fooport
toolrepo:
hitting the toolfound vs toolrepo check
Installing package: fooport
Usage error: please install using base project name without port suffix.
  try: zopen install foo

Trying with some unknown repo

$ zopen install foo
- Querying remote repo for latest package information
chosenRepo: foo
toolrepo: foo
***ERROR: The following requested port(s) do not exist:
	foo

I'm not certain what the first clause is doing, but without the group, the toolrepo is mostly blank. I thought this would at least provide a response.

I looked to verify the if = clause check vs if ==, this is indeed the original code. I'm not certain if its valid or not. If its invalid or needs comment, i propose we move to a new issue. See here

[netskink]

Regarding the multiple install issue

With the patch in pull request present:

$ zopen install findutils git
- Querying remote repo for latest package information
chosenRepo: findutils
toolrepo: findutils
hitting the toolfound vs toolrepo check
chosenRepo: git
toolrepo: git
hitting the toolfound vs toolrepo check
Installing package: findutils
- Replacing findutils version '4.9.0.20231109_042435' with '4.9.0.20231115_215243'
- Found existing file 'findutils-4.9.0.20231115_215243.zos.pax.Z' in zopen package cache
- Processing findutils-4.9.0.20231115_215243.zos.pax.Z...
After this operation, 5.14 MB of additional disk space will be used.
Do you want to continue? [y/n]

[netskink]

with the original code present, it looks like this:

$ zopen install findutils git
- Querying remote repo for latest package information
chosenRepo: findutils
toolrepo: findutils
hitting the toolfound vs toolrepo check
chosenRepo: git
toolrepo: git
hitting the toolfound vs toolrepo check
Installing package: findutils
- Replacing findutils version '4.9.0.20231109_042435' with '4.9.0.20231115_215243'
- Found existing file 'findutils-4.9.0.20231115_215243.zos.pax.Z' in zopen package cache
- Processing findutils-4.9.0.20231115_215243.zos.pax.Z...
After this operation, 5.14 MB of additional disk space will be used.
Do you want to continue? [y/n]

[netskink]

with patch present for multiple installs and port suffix added

$ zopen install findutilsport gitport
- Querying remote repo for latest package information
chosenRepo: findutilsport
toolrepo: findutils
hitting the toolfound vs toolrepo check
chosenRepo: gitport
toolrepo: git
hitting the toolfound vs toolrepo check
Installing package: findutilsport
Usage error: please install using base project name without port suffix.
  try: zopen install findutils

[netskink]

with original code

(Note the check at top is still present, just the regex with sed grouping is not)

$ zopen install findutilsport gitport
- Querying remote repo for latest package information
chosenRepo: findutilsport
toolrepo:
hitting the toolfound vs toolrepo check
chosenRepo: gitport
toolrepo:
hitting the toolfound vs toolrepo check
Installing package: findutilsport
Usage error: please install using base project name without port suffix.
  try: zopen install findutils

[netskink]

Igor, regarding:

Just to confirm, if you do this:

zopen install --reinstall git findutilsport

will it install git first and then complain about findutilsport?

Yes.

$ zopen install --reinstall git findutilsport
- Querying remote repo for latest package information
chosenRepo: git
toolrepo: git
hitting the toolfound vs toolrepo check
chosenRepo: findutilsport
toolrepo: findutils
hitting the toolfound vs toolrepo check
Installing package: git
- Replacing git version '2.41.0.20231023_154410' with '2.43.0.20231127_145951'
- Downloading git-2.43.0.20231127_145951.zos.pax.Z file from remote to zopen package cache...
- Downloaded git-2.43.0.20231127_145951.zos.pax.Z file from remote to zopen package cache.
- Processing git-2.43.0.20231127_145951.zos.pax.Z...
After this operation, 106.41 MB of additional disk space will be used.
Do you want to continue? [y/n]
y
- Expanding git-2.43.0.20231127_145951.zos.pax.Z
- Expanded
- Integration complete
- Checking for env file
- .env file found, adding to profiled processing
- Sourcing environment to run any setup
Setting up git...
Setup completed.
- Checking for obsoleted files in /z/jd895801/zopen/usr/ tree from git
- Checking 604 potential links
- Complete
- Checking 73 dir links
Successfully installed git
Installing package: findutilsport
Usage error: please install using base project name without port suffix.
  try: zopen install findutils

[IgorTodorovskiIBM]

Igor, regarding:

Just to confirm, if you do this:

zopen install --reinstall git findutilsport

will it install git first and then complain about findutilsport?

Yes.

$ zopen install --reinstall git findutilsport
- Querying remote repo for latest package information
chosenRepo: git
toolrepo: git
hitting the toolfound vs toolrepo check
chosenRepo: findutilsport
toolrepo: findutils
hitting the toolfound vs toolrepo check
Installing package: git
- Replacing git version '2.41.0.20231023_154410' with '2.43.0.20231127_145951'
- Downloading git-2.43.0.20231127_145951.zos.pax.Z file from remote to zopen package cache...
- Downloaded git-2.43.0.20231127_145951.zos.pax.Z file from remote to zopen package cache.
- Processing git-2.43.0.20231127_145951.zos.pax.Z...
After this operation, 106.41 MB of additional disk space will be used.
Do you want to continue? [y/n]
y
- Expanding git-2.43.0.20231127_145951.zos.pax.Z
- Expanded
- Integration complete
- Checking for env file
- .env file found, adding to profiled processing
- Sourcing environment to run any setup
Setting up git...
Setup completed.
- Checking for obsoleted files in /z/jd895801/zopen/usr/ tree from git
- Checking 604 potential links
- Complete
- Checking 73 dir links
Successfully installed git
Installing package: findutilsport
Usage error: please install using base project name without port suffix.
  try: zopen install findutils

Ok, thanks for verifying. Let's open an issue to track that. I'll merge this PR in the meantime.

[netskink]

Thanks for merging. Good idea on a new PR. Please assign to me since I don’t quite follow what is the follow up problem. A fresh issue should convey the remaining issue.