util: makeIssue.sh considers WORK_HOME

[openroad-ci]

Currently, make issues are always saved at <git_root>/flow, even when WORK_HOME is set to somewhere else, this PR makes the make issue behave the same as other generated files like logs, reports, objects, etc

[oharboe]

there seems to be a lot of drive by fixes here, but it us unclear what the motivation is and what is being solved.

This makes it hard to review this change: it is not a single concern.

I dont think creating lots of variables helps readability, unsure what the motivation is.

[oharboe]

Also, there is very limited test coverage for all the use cases for make issue, so we should be wary of making changes unless we have a specific problem we are changing. We could add more automated testing....

[vvbandeira]

there seems to be a lot of drive by fixes here, but it us unclear what the motivation is and what is being solved.

The single goal of the PR is to make the make <script>_issue rule respect the WORK_HOME variable.

Before this PR, whenever you set WORK_HOME to a path other than <git_root>/flow, all generated files (e.g., logs, results, objects, ...) were saved in WORK_HOME. That was not the case for issue files; issue tar files would still be saved in the <git_root>/flow, and there was no way to change that with variables.

This makes it hard to review this change: it is not a single concern.

The changes to flow/test/test_make_issue.sh were needed because it was broken even before the PR; now it is not. It looked like a good time to fix the test since I was working on what the test checks for. I can split the fix into another PR if that helps.

I dont think creating lots of variables helps readability, unsure what the motivation is.

Not sure what variables you are referring to, but the goal of adding the variables that I added:

• $ISSUE_TARGET and $ISSUE_DEST are needed because we need to manipulate the input $1. I also think not using $1 is a readability improvement anyway.
• Replacing multiple occurrences of the same text with a variable reduces the probability of forgetting to change all the occurrences.
• Moving $TARGET and $ISSUE_TARGET is a first step to make the script less hardcoded. I can revert the $TARGET variable, but $ISSUE_TARGET also falls on the "multiple occurrences" from the previous item.

Also, there is very limited test coverage for all the use cases for make issue, so we should be wary of making changes unless we have a specific problem we are changing.

I need this feature because I have run into a situation where I don't have write access to <git_root>/flow. Changing WORK_HOME works for running the flow, but not for saving issues.

We could add more automated testing....

If you can describe in detail the other use cases that the current test_make_issue.sh does not cover, I can work and implement more checks.

[oharboe]

no further comments, I will try it once merged, didnt review in detail, would need to try it.

[vvbandeira]

I am ok with waiting for you to test. This is not a huge blocker at the moment.

[oharboe]

I am ok with waiting for you to test. This is not a huge blocker at the moment.

I am sure it will work, I will try it out of the docker image, so merging makes testing quicker.