Move products to a dedicated folder

[matejak]

This PR aims to make the project directory tree more structured.
As it just moves directories, the backward compatibility of patches can be achieved by creating symlinks from the root directory:

for prod in products/*; do ln -s "$prod" $(basename "$prod"); done

[openscap-ci]

Changes identified:
Others:
 Changes in Python files.

Show details

Others:
 Python abstract syntax tree change found in build-scripts/build_rule_playbooks.py.
 Python abstract syntax tree change found in ssg/products.py.
 Python abstract syntax tree change found in tests/ssg_test_suite/common.py.
 Python abstract syntax tree change found in tests/test_machine_only_rules.py.
 Python abstract syntax tree change found in tests/test_parse_affected.py.
 Python abstract syntax tree change found in tests/test_parse_platform.py.
 Python abstract syntax tree change found in tests/unit/ssg-module/test_controls.py.
 Python abstract syntax tree change found in utils/rule_dir_json.py.

Recommended tests to execute:
 (cd build && cmake ../ && ctest -j4)

[JAORMX]

This change makes sense to me and it does make it easier to view the relevant covered products. Let's wait for more folks from the community to give feedback on this. I'd also like to get some time to change the OCP4 CI tests to reflect this new structure before merging this, so we don't have a long breakage.

[carlosmmatos]

@matejak thanks for the PR, I've been advocating for a change like this to clean up the structure of the project. I think this will make things easier to digest at first glance, and hopefully easier to follow for new contributors.

I also agree with @JAORMX that we should get more feedback from the community members to see if this is the appropriate way to clean up the structure, or perhaps there are other ideas.

[matejak]

I completely agree to take this one slow. I think that we can leave it open at least until the end of May to accumulate feedback, and if the proposal is still alive, then I will resolve confilcts and we can then consider merging this.

[carlosmmatos]

I completely agree to take this one slow. I think that we can leave it open at least until the end of May to accumulate feedback, and if the proposal is still alive, then I will resolve confilcts and we can then consider merging this.

Do you want to add this to the discussions?

[matejak]

Do you want to add this to the discussions?

I don't know, and I am leaning towards that it isn't needed - the discussion itself can take place here, and I have announced the intention on the mailing list, which is a traditional communication channel. However, if there is no feedback next week, I think that we can announce in the GH discussion.

[JAORMX]

/retest

[JAORMX]

/retest

[JAORMX]

/retest

[matejak]

/retest

[matejak]

I think that as no objections came, it is time to prepare for start seriously considering merging this PR.

[matejak]

/retest

[matejak]

/retest

[cipherboy]

@matejak: One suggestion by @richardmaciel-canonical was that if you used git mv you'd get a cleaner diff and patches would apply cleanly. Just a suggestion, since I noticed there are conflicts.

[matejak]

@matejak: One suggestion by @richardmaciel-canonical was that if you used git mv you'd get a cleaner diff and patches would apply cleanly. Just a suggestion, since I noticed there are conflicts.

I think that I have used git mv, and rebasing to solve conflicts is trivial - I never had to edit anything. So I suspect that git is not confused by the move, but Github is.

[JAORMX]

/retest

[JAORMX]

/test e2e-aws-ocp4-e8

[matejak]

Note for reviewers - the git diff (not Github diff) is very clean except files that had to be modified because they contained relative paths (s.a. XSLT transforms).

[ggbecker]

I'm getting this warning when running ./build_product rhel8 and there are no symlinks to products folder in the root directory:

CMake Warning (dev):
  Policy CMP0058 is not set: Ninja requires custom command byproducts to be
  explicit.  Run "cmake --help-policy CMP0058" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.

  This project specifies custom command DEPENDS on files in the build tree
  that are not specified as the OUTPUT or BYPRODUCTS of any
  add_custom_command or add_custom_target:

   rhosp13
   rhv4

  For compatibility with versions of CMake that did not have the BYPRODUCTS
  option, CMake is generating phony rules for such files to convince 'ninja'
  to build.

  Project authors should add the missing BYPRODUCTS or OUTPUT options to the
  custom commands that produce these files.
This warning is for project developers.  Use -Wno-dev to suppress it.

If I create the symlinks for these two folder in the root directory then the warning disappears. Do you know what could that be? Do you see the same on your side?

It looks like it's a valid issue: https://github.com/ComplianceAsCode/content/runs/2792925585?check_suite_focus=true#step:4:110

Update1: I'm suspecting that has something to do with these lines: https://github.com/ComplianceAsCode/content/blob/master/cmake/SSGCommon.cmake#L1405-L1408

Update2: Prepending products/ to those products seems to fix the issues, but I'm not 100% sure if that's the correct fix.

[JAORMX]

/retest

[ggbecker]

LGTM. The commits were accidentally pushed directly to master. No need to reopen nor revert. The changes in master look fine. So no need to take any extra action.

[openshift-ci]

@matejak: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-ocp4-moderate	822cced	link	/test e2e-aws-ocp4-moderate
ci/prow/e2e-aws-rhcos4-moderate	822cced	link	/test e2e-aws-rhcos4-moderate

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.